Greg, On Thu, Aug 19, 2010 at 11:07 PM, Randy Dunlap <randy.dunlap@xxxxxxxxxx> wrote: > On 08/19/10 10:35, Savoy, Pavan wrote: >> Randy, >> >> >>> -----Original Message----- >>> From: Randy Dunlap [mailto:randy.dunlap@xxxxxxxxxx] >>> Sent: Thursday, August 19, 2010 12:32 PM >>> To: Savoy, Pavan >>> Cc: gregkh@xxxxxxx; alan@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; >>> devel@xxxxxxxxxxxxxxxxxxxx; Jain, Naveen >>> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device >>> >>> On 08/19/10 11:08, pavan_savoy@xxxxxx wrote: >>>> From: Pavan Savoy <pavan_savoy@xxxxxx> >>>> >>>> In order to support multiple ST platform devices, a new symbol >>>> 'st_get_plat_device' earlier needed to be exported by the arch/XX/brd-XX.c >>>> file which intends to add the ST platform device. >>>> >>>> On removing this dependency, now inside ST driver maintain the array of >>>> ST platform devices that would be registered. >>>> As of now let id=0, as and when we end up having such platforms >>>> where mutliple ST devices can exist, id would come from >>>> protocol drivers (BT, FM and GPS) as to on which platform device >>>> they want to register to. >>>> >>>> Signed-off-by: Pavan Savoy <pavan_savoy@xxxxxx> >>> >>> Thanks, that builds cleanly. I'm OK with it if you are comfortable with a >>> hard limit on the fixed number of devices that can be supported: >> >> Yep, Thanks for pointing out, sort of cleaned up the code. >> >>> +#define MAX_ST_DEVICES 3 /* Imagine 1 on each UART for now */ >>> +struct platform_device *st_kim_devices[MAX_ST_DEVICES]; >>> >>> We usually try not to have such limits nor use arrays like that, >>> but if the nature of the device and its platform/environment is like >>> that, so be it. >>> >> >> Actually on all platforms that I have seen there's only 1 such device. >> The device is basically a connectivity chip (with Bluetooth, FM and GPS working >> on a single UART) >> >> The number which I mentioned was out of imagination. >> I don't think we are ever going to have multiple of them either... > > OK, thanks. > > Acked-by: Randy Dunlap <randy.dunlap@xxxxxxxxxx> Can you please merge this patch ? Also please have a look at the driver and suggest what needs to be done to move it out of staging. >> >>>> --- >>>> drivers/staging/ti-st/st.h | 1 - >>>> drivers/staging/ti-st/st_core.c | 9 ++++----- >>>> drivers/staging/ti-st/st_core.h | 2 +- >>>> drivers/staging/ti-st/st_kim.c | 22 +++++++++++++++++++--- >>>> 4 files changed, 24 insertions(+), 10 deletions(-) >>>> >>>> diff --git a/drivers/staging/ti-st/st.h b/drivers/staging/ti-st/st.h >>>> index 9952579..1b3060e 100644 >>>> --- a/drivers/staging/ti-st/st.h >>>> +++ b/drivers/staging/ti-st/st.h >>>> @@ -80,5 +80,4 @@ struct st_proto_s { >>>> extern long st_register(struct st_proto_s *); >>>> extern long st_unregister(enum proto_type); >>>> >>>> -extern struct platform_device *st_get_plat_device(void); >>>> #endif /* ST_H */ >>>> diff --git a/drivers/staging/ti-st/st_core.c b/drivers/staging/ti- >>> st/st_core.c >>>> index 063c9b1..b85d8bf 100644 >>>> --- a/drivers/staging/ti-st/st_core.c >>>> +++ b/drivers/staging/ti-st/st_core.c >>>> @@ -38,7 +38,6 @@ >>>> #include "st_ll.h" >>>> #include "st.h" >>>> >>>> -#define VERBOSE >>>> /* strings to be used for rfkill entries and by >>>> * ST Core to be used for sysfs debug entry >>>> */ >>>> @@ -581,7 +580,7 @@ long st_register(struct st_proto_s *new_proto) >>>> long err = 0; >>>> unsigned long flags = 0; >>>> >>>> - st_kim_ref(&st_gdata); >>>> + st_kim_ref(&st_gdata, 0); >>>> pr_info("%s(%d) ", __func__, new_proto->type); >>>> if (st_gdata == NULL || new_proto == NULL || new_proto->recv == NULL >>>> || new_proto->reg_complete_cb == NULL) { >>>> @@ -713,7 +712,7 @@ long st_unregister(enum proto_type type) >>>> >>>> pr_debug("%s: %d ", __func__, type); >>>> >>>> - st_kim_ref(&st_gdata); >>>> + st_kim_ref(&st_gdata, 0); >>>> if (type < ST_BT || type >= ST_MAX) { >>>> pr_err(" protocol %d not supported", type); >>>> return -EPROTONOSUPPORT; >>>> @@ -767,7 +766,7 @@ long st_write(struct sk_buff *skb) >>>> #endif >>>> long len; >>>> >>>> - st_kim_ref(&st_gdata); >>>> + st_kim_ref(&st_gdata, 0); >>>> if (unlikely(skb == NULL || st_gdata == NULL >>>> || st_gdata->tty == NULL)) { >>>> pr_err("data/tty unavailable to perform write"); >>>> @@ -818,7 +817,7 @@ static int st_tty_open(struct tty_struct *tty) >>>> struct st_data_s *st_gdata; >>>> pr_info("%s ", __func__); >>>> >>>> - st_kim_ref(&st_gdata); >>>> + st_kim_ref(&st_gdata, 0); >>>> st_gdata->tty = tty; >>>> tty->disc_data = st_gdata; >>>> >>>> diff --git a/drivers/staging/ti-st/st_core.h b/drivers/staging/ti- >>> st/st_core.h >>>> index e0c32d1..8601320 100644 >>>> --- a/drivers/staging/ti-st/st_core.h >>>> +++ b/drivers/staging/ti-st/st_core.h >>>> @@ -117,7 +117,7 @@ int st_core_init(struct st_data_s **); >>>> void st_core_exit(struct st_data_s *); >>>> >>>> /* ask for reference from KIM */ >>>> -void st_kim_ref(struct st_data_s **); >>>> +void st_kim_ref(struct st_data_s **, int); >>>> >>>> #define GPS_STUB_TEST >>>> #ifdef GPS_STUB_TEST >>>> diff --git a/drivers/staging/ti-st/st_kim.c b/drivers/staging/ti-st/st_kim.c >>>> index b4a6c7f..9e99463 100644 >>>> --- a/drivers/staging/ti-st/st_kim.c >>>> +++ b/drivers/staging/ti-st/st_kim.c >>>> @@ -72,11 +72,26 @@ const unsigned char *protocol_names[] = { >>>> PROTO_ENTRY(ST_GPS, "GPS"), >>>> }; >>>> >>>> +#define MAX_ST_DEVICES 3 /* Imagine 1 on each UART for now */ >>>> +struct platform_device *st_kim_devices[MAX_ST_DEVICES]; >>>> >>>> /**********************************************************************/ >>>> /* internal functions */ >>>> >>>> /** >>>> + * st_get_plat_device - >>>> + * function which returns the reference to the platform device >>>> + * requested by id. As of now only 1 such device exists (id=0) >>>> + * the context requesting for reference can get the id to be >>>> + * requested by a. The protocol driver which is registering or >>>> + * b. the tty device which is opened. >>>> + */ >>>> +static struct platform_device *st_get_plat_device(int id) >>>> +{ >>>> + return st_kim_devices[id]; >>>> +} >>>> + >>>> +/** >>>> * validate_firmware_response - >>>> * function to return whether the firmware response was proper >>>> * in case of error don't complete so that waiting for proper >>>> @@ -353,7 +368,7 @@ void st_kim_chip_toggle(enum proto_type type, enum >>> kim_gpio_state state) >>>> struct kim_data_s *kim_gdata; >>>> pr_info(" %s ", __func__); >>>> >>>> - kim_pdev = st_get_plat_device(); >>>> + kim_pdev = st_get_plat_device(0); >>>> kim_gdata = dev_get_drvdata(&kim_pdev->dev); >>>> >>>> if (kim_gdata->gpios[type] == -1) { >>>> @@ -574,12 +589,12 @@ static int kim_toggle_radio(void *data, bool blocked) >>>> * This would enable multiple such platform devices to exist >>>> * on a given platform >>>> */ >>>> -void st_kim_ref(struct st_data_s **core_data) >>>> +void st_kim_ref(struct st_data_s **core_data, int id) >>>> { >>>> struct platform_device *pdev; >>>> struct kim_data_s *kim_gdata; >>>> /* get kim_gdata reference from platform device */ >>>> - pdev = st_get_plat_device(); >>>> + pdev = st_get_plat_device(id); >>>> kim_gdata = dev_get_drvdata(&pdev->dev); >>>> *core_data = kim_gdata->core_data; >>>> } >>>> @@ -623,6 +638,7 @@ static int kim_probe(struct platform_device *pdev) >>>> long *gpios = pdev->dev.platform_data; >>>> struct kim_data_s *kim_gdata; >>>> >>>> + st_kim_devices[pdev->id] = pdev; >>>> kim_gdata = kzalloc(sizeof(struct kim_data_s), GFP_ATOMIC); >>>> if (!kim_gdata) { >>>> pr_err("no mem to allocate"); >>> >>> >>> -- >>> ~Randy >>> *** Remember to use Documentation/SubmitChecklist when testing your code *** > > > -- > ~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/ > > > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
