Re: [PATCH 0/3] drivers:staging:ti-st: patches

[Pavan Savoy]

Randy,

On Thu, Aug 19, 2010 at 4:26 AM, Randy Dunlap <randy.dunlap@xxxxxxxxxx> wrote:
> On 08/18/10 15:46, Savoy, Pavan wrote:
>> Randy,
>>
>>
>>> -----Original Message-----
>>> From: Randy Dunlap [mailto:randy.dunlap@xxxxxxxxxx]
>>> Sent: Wednesday, August 18, 2010 4:05 PM
>>> To: Savoy, Pavan
>>> Cc: devel@xxxxxxxxxxxxxxxxxxxx; gregkh@xxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
>>> alan@xxxxxxxxxxxxxxxxxxx
>>> Subject: Re: [PATCH 0/3] drivers:staging:ti-st: patches
>>>
>>> On Mon, 26 Jul 2010 10:37:02 +0530 Pavan Savoy wrote:
>>>
>>>> On Fri, Jul 23, 2010 at 8:47 PM, Randy Dunlap <randy.dunlap@xxxxxxxxxx>
>>> wrote:
>>>>> On 07/22/10 21:56, Pavan Savoy wrote:
>>>>>> Randy,
>>>>>>
>>>>>> On Fri, Jul 23, 2010 at 1:23 AM, Randy Dunlap <randy.dunlap@xxxxxxxxxx>
>>> wrote:
>>>>>>> On Thu, 22 Jul 2010 05:32:04 -0500 pavan_savoy@xxxxxx wrote:
>>>>>>>
>>>>>>>> From: Pavan Savoy <pavan_savoy@xxxxxx>
>>>>>>>>
>>>>>>>> The following patches cleanup bit of a mess and also adds functionality
>>> to protocol drivers.
>>>>>>>> with the 3rd patch now providing context to even the protocol drivers,
>>> the single device limit
>>>>>>>> or support for multiple devices would be easier to implement.
>>>>>>>>
>>>>>>>> These patches depend on the previously submitted
>>>>>>>> 0001-drivers-staging-ti-st-make-use-of-linux-err-codes.patch
>>>>>>>> commit d39d49b393d94f4137cee4f64526a4695352f183
>>>>>>>>
>>>>>>>> Pavan Savoy (3):
>>>>>>>> Â drivers:staging:ti-st: smarten, reduce logs
>>>>>>>> Â drivers:staging:ti-st: cleanup code comments
>>>>>>>> Â drivers:staging:ti-st: give proto drivers context
>>>>>>>>
>>>>>>>> Âdrivers/staging/ti-st/bt_drv.c Â| Â 23 +++++---
>>>>>>>> Âdrivers/staging/ti-st/st.h   Â|  52 +++++++++--------
>>>>>>>> Âdrivers/staging/ti-st/st_core.c | Â118 +++++++++++++++++++------------
>>> --------
>>>>>>>> Âdrivers/staging/ti-st/st_core.h | Â 74 +++++++++++++++++--------
>>>>>>>> Âdrivers/staging/ti-st/st_kim.c Â| Â 73 ++++++++++++++----------
>>>>>>>> Âdrivers/staging/ti-st/st_kim.h Â| Â 77 ++++++++++++++++---------
>>>>>>>> Âdrivers/staging/ti-st/st_ll.c  |  Â4 +-
>>>>>>>> Âdrivers/staging/ti-st/st_ll.h  |  Â9 +++-
>>>>>>>> Â8 files changed, 255 insertions(+), 175 deletions(-)
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> I have reported this error a few times. ÂWhere is the patch for it??
>>>>>>>
>>>>>>> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!
>>>>>>
>>>>>>
>>>>>> Yes, on one of the earlier patch sets, I had mentioned that the ST
>>>>>> driver being a platform device, needs definition in any of the
>>>>>> arch/XX/mach-XX/board-XX.c or devices.c or somewhere...
>>>>>>
>>>>>> and hence it is in that board-XX.c file that the symbol
>>>>>> st_get_plat_device needs to be exported, the reason for that being,
>>>>>>
>>>>>> ST driver being both a TTY ldisc driver and platform driver, in TTY
>>>>>> contexts it would need to refer to platform driver's data. So it does
>>>>>> a st_get_plat_device which returns the platform device structure, and
>>>>>> then does a dev_getdrvdata from it.
>>>>>>
>>>>>> here's a snippet of code ...
>>>>>> /*
>>>>>> Â* ST related functions related functions
>>>>>> Â*/
>>>>>> #include <linux/platform_device.h>
>>>>>>
>>>>>> long gpios[] = { 55, -1, -1 };
>>>>>> static struct platform_device ti_st_device = {
>>>>>>     .name      = "kim",
>>>>>>     .id       = -1,
>>>>>>     .dev.platform_data   Â= &gpios,
>>>>>> };
>>>>>>
>>>>>> struct platform_device *st_get_plat_device(void)
>>>>>> {
>>>>>> Â Â Â Â return &ti_st_device;
>>>>>> }
>>>>>> EXPORT_SYMBOL(st_get_plat_device);
>>>>>>
>>>>>> static __init int add_ti_st_device(void)
>>>>>> {
>>>>>> Â Â Â Â platform_device_register(&ti_st_device);
>>>>>> Â Â Â Â dev_info(&ti_st_device.dev,"registered platform TI ST device\n");
>>>>>>
>>>>>> Â Â Â Â return 0;
>>>>>> }
>>>>>> device_initcall(add_ti_st_device);
>>>>>>
>>>>>>
>>>>>> We have that in our local trees in arch/arm/mach-omap2/board-sdp4430.c
>>>>>
>>>>> Thanks for the explanation.
>>>>>
>>>>> Is the driver platform-specific?
>>>>> E.g., should it not even be built on x86?
>>>>
>>>> Yes. Requirement of the hardware is very much a must.
>>>> However it is a separate peripheral (WiLink 7 - uart interfaced), may
>>>> be there is a x86 platform with this - but certainly not desktops.
>>>>
>>>> on linux-next, I generally put in that st_dev.c file for x86 - verify
>>>> whether it builds as a module, inserts/rmmod, basic other
>>>> functionalities (which doesn't involve response from chip..)
>>>> But verify full functionality on board which constitutes that.
>>>
>>> Hi,
>>> Please make this driver build cleanly on X86 or prevent it from being built
>>> there.
>>
>> Do you do something like a make allyesconfig?
>
> I do 50 X86 randconfig builds per night (scripted, while I sleep).
> As long as STAGING is enabled and RFKILL is enabled, then TI_ST can be enabled,
> so usually it fails to build.
>
>> I am having a hard time figuring out why is it building for you in the first place?
>> make defconfig doesn't build it, neither does make i386_defconfig.
>
> That is irrelevant. ÂIt's now in mainline and it is causing build failures.
>
>> May be a patch which does 'default N' in drivers/staging/ti-st/Kconfig, would suffice?
>
> Nope.
>
>> Please suggest...
>
>>>>>>> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!
>
> st_get_plat_device will have to be defined for any build platform,
> or not referenced, or make it not possible to enable TI_ST unless
> st_get_plat_device is defined.

I was thinking as to why do I even have this platform device reference
via an exported symbol,
and thanks for pointing out, I don't even need this function
"st_get_plat_device()".
I already have the platform_device reference in my _probe function !!!..
I will send out a patch 2morrow.

This actually sorts of clarifies how I  support multiple devices too...
This is how my probe would then look like ...
static int kim_probe(struct platform_device *pdev)
{
....
...
platform_devices_i_have[id] = pdev;
.....
....
}

and st_get_plat_device(int id) would be defined as...
{
return platform_devices_i_have[id];
}

Does this sound ok ?


> --
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
> Please read the FAQ at Âhttp://www.tux.org/lkml/
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/