Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Randy Dunlap]

On 08/19/10 11:08, pavan_savoy@ti.com wrote:
> From: Pavan Savoy <pavan_savoy@ti.com>
> 
> 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@ti.com>

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:

+#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.


> ---
>  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 ***

RE: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Savoy, Pavan]

Randy,
 

> -----Original Message-----
> From: Randy Dunlap [mailto:randy.dunlap@oracle.com]
> Sent: Thursday, August 19, 2010 12:32 PM
> To: Savoy, Pavan
> Cc: gregkh@suse.de; alan@lxorguk.ukuu.org.uk; linux-kernel@vger.kernel.org;
> devel@driverdev.osuosl.org; Jain, Naveen
> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
> 
> On 08/19/10 11:08, pavan_savoy@ti.com wrote:
> > From: Pavan Savoy <pavan_savoy@ti.com>
> >
> > 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@ti.com>
> 
> 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...

> > ---
> >  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 ***

Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Randy Dunlap]

On 08/19/10 10:35, Savoy, Pavan wrote:
> Randy,
>  
> 
>> -----Original Message-----
>> From: Randy Dunlap [mailto:randy.dunlap@oracle.com]
>> Sent: Thursday, August 19, 2010 12:32 PM
>> To: Savoy, Pavan
>> Cc: gregkh@suse.de; alan@lxorguk.ukuu.org.uk; linux-kernel@vger.kernel.org;
>> devel@driverdev.osuosl.org; Jain, Naveen
>> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
>>
>> On 08/19/10 11:08, pavan_savoy@ti.com wrote:
>>> From: Pavan Savoy <pavan_savoy@ti.com>
>>>
>>> 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@ti.com>
>>
>> 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@oracle.com>


> 
>>> ---
>>>  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 ***

[PATCH] drivers:staging:ti-st: remove st_get_plat_device

[pavan_savoy]

From: Pavan Savoy <pavan_savoy@ti.com>

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@ti.com>
---
 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");
-- 
1.6.5

Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Pavan Savoy]

Greg,

On Thu, Aug 19, 2010 at 11:07 PM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> On 08/19/10 10:35, Savoy, Pavan wrote:
>> Randy,
>>
>>
>>> -----Original Message-----
>>> From: Randy Dunlap [mailto:randy.dunlap@oracle.com]
>>> Sent: Thursday, August 19, 2010 12:32 PM
>>> To: Savoy, Pavan
>>> Cc: gregkh@suse.de; alan@lxorguk.ukuu.org.uk; linux-kernel@vger.kernel.org;
>>> devel@driverdev.osuosl.org; Jain, Naveen
>>> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
>>>
>>> On 08/19/10 11:08, pavan_savoy@ti.com wrote:
>>>> From: Pavan Savoy <pavan_savoy@ti.com>
>>>>
>>>> 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@ti.com>
>>>
>>> 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@oracle.com>


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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
>
>

Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Greg KH]

On Sun, Aug 22, 2010 at 07:20:00AM +0530, Pavan Savoy wrote:
> Greg,
> 
> On Thu, Aug 19, 2010 at 11:07 PM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> > On 08/19/10 10:35, Savoy, Pavan wrote:
> >> Randy,
> >>
> >>
> >>> -----Original Message-----
> >>> From: Randy Dunlap [mailto:randy.dunlap@oracle.com]
> >>> Sent: Thursday, August 19, 2010 12:32 PM
> >>> To: Savoy, Pavan
> >>> Cc: gregkh@suse.de; alan@lxorguk.ukuu.org.uk; linux-kernel@vger.kernel.org;
> >>> devel@driverdev.osuosl.org; Jain, Naveen
> >>> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
> >>>
> >>> On 08/19/10 11:08, pavan_savoy@ti.com wrote:
> >>>> From: Pavan Savoy <pavan_savoy@ti.com>
> >>>>
> >>>> 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@ti.com>
> >>>
> >>> 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@oracle.com>
> 
> 
> 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.

Yes, I will work on it, sorry, been swamped with other work lately.

Don't worry, this patch is in my "to-apply" queue.

thanks,

greg k-h

RE: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Savoy, Pavan]

Greg,



> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Saturday, August 21, 2010 10:33 PM
> To: Savoy, Pavan
> Cc: Randy Dunlap; alan@lxorguk.ukuu.org.uk; linux-kernel@vger.kernel.org;
> devel@driverdev.osuosl.org; Jain, Naveen
> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
> 
> On Sun, Aug 22, 2010 at 07:20:00AM +0530, Pavan Savoy wrote:
> > Greg,
> >
> > On Thu, Aug 19, 2010 at 11:07 PM, Randy Dunlap <randy.dunlap@oracle.com>
> wrote:
> > > On 08/19/10 10:35, Savoy, Pavan wrote:
> > >> Randy,
> > >>
> > >>
> > >>> -----Original Message-----
> > >>> From: Randy Dunlap [mailto:randy.dunlap@oracle.com]
> > >>> Sent: Thursday, August 19, 2010 12:32 PM
> > >>> To: Savoy, Pavan
> > >>> Cc: gregkh@suse.de; alan@lxorguk.ukuu.org.uk; linux-
> kernel@vger.kernel.org;
> > >>> devel@driverdev.osuosl.org; Jain, Naveen
> > >>> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
> > >>>
> > >>> On 08/19/10 11:08, pavan_savoy@ti.com wrote:
> > >>>> From: Pavan Savoy <pavan_savoy@ti.com>
> > >>>>
> > >>>> 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@ti.com>
> > >>>
> > >>> 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@oracle.com>
> >
> >
> > 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.
> 
> Yes, I will work on it, sorry, been swamped with other work lately.
> 
> Don't worry, this patch is in my "to-apply" queue.

Any update on this?
Apologize for being annoying, but some patch traffic on the driver is calming :)


> thanks,
> 
> greg k-h

Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Greg KH]

On Tue, Aug 31, 2010 at 03:39:00AM +0530, Savoy, Pavan wrote:
> Greg,
> 
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@suse.de]
> > Sent: Saturday, August 21, 2010 10:33 PM
> > To: Savoy, Pavan
> > Cc: Randy Dunlap; alan@lxorguk.ukuu.org.uk; linux-kernel@vger.kernel.org;
> > devel@driverdev.osuosl.org; Jain, Naveen
> > Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
> > 
> > On Sun, Aug 22, 2010 at 07:20:00AM +0530, Pavan Savoy wrote:
> > > Greg,
> > >
> > > On Thu, Aug 19, 2010 at 11:07 PM, Randy Dunlap <randy.dunlap@oracle.com>
> > wrote:
> > > > On 08/19/10 10:35, Savoy, Pavan wrote:
> > > >> Randy,
> > > >>
> > > >>
> > > >>> -----Original Message-----
> > > >>> From: Randy Dunlap [mailto:randy.dunlap@oracle.com]
> > > >>> Sent: Thursday, August 19, 2010 12:32 PM
> > > >>> To: Savoy, Pavan
> > > >>> Cc: gregkh@suse.de; alan@lxorguk.ukuu.org.uk; linux-
> > kernel@vger.kernel.org;
> > > >>> devel@driverdev.osuosl.org; Jain, Naveen
> > > >>> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
> > > >>>
> > > >>> On 08/19/10 11:08, pavan_savoy@ti.com wrote:
> > > >>>> From: Pavan Savoy <pavan_savoy@ti.com>
> > > >>>>
> > > >>>> 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@ti.com>
> > > >>>
> > > >>> 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@oracle.com>
> > >
> > >
> > > 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.
> > 
> > Yes, I will work on it, sorry, been swamped with other work lately.
> > 
> > Don't worry, this patch is in my "to-apply" queue.
> 
> Any update on this?

Nope, sorry.  I got sick and combined with vacation requirements from
the family, and my "real job" and other things lately, I hope to get to
it by the end of this week.  If not, kick me, I'll have time on a
trans-continent flight next week that should give me plenty of time to
do this...

thanks,

greg k-h

RE: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Savoy, Pavan]

Greg,

> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Monday, August 30, 2010 11:11 PM
> To: Savoy, Pavan
> Cc: linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org
> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
> 
> On Tue, Aug 31, 2010 at 03:39:00AM +0530, Savoy, Pavan wrote:
> > Greg,
> >
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@suse.de]
> > > Sent: Saturday, August 21, 2010 10:33 PM
> > > To: Savoy, Pavan
> > > Cc: Randy Dunlap; alan@lxorguk.ukuu.org.uk; linux-kernel@vger.kernel.org;
> > > devel@driverdev.osuosl.org; Jain, Naveen
> > > Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
> > >
> > > On Sun, Aug 22, 2010 at 07:20:00AM +0530, Pavan Savoy wrote:
> > > > Greg,
> > > >
> > > > On Thu, Aug 19, 2010 at 11:07 PM, Randy Dunlap <randy.dunlap@oracle.com>
> > > wrote:
> > > > > On 08/19/10 10:35, Savoy, Pavan wrote:
> > > > >> Randy,
> > > > >>
> > > > >>
> > > > >>> -----Original Message-----
> > > > >>> From: Randy Dunlap [mailto:randy.dunlap@oracle.com]
> > > > >>> Sent: Thursday, August 19, 2010 12:32 PM
> > > > >>> To: Savoy, Pavan
> > > > >>> Cc: gregkh@suse.de; alan@lxorguk.ukuu.org.uk; linux-
> > > kernel@vger.kernel.org;
> > > > >>> devel@driverdev.osuosl.org; Jain, Naveen
> > > > >>> Subject: Re: [PATCH] drivers:staging:ti-st: remove
> st_get_plat_device
> > > > >>>
> > > > >>> On 08/19/10 11:08, pavan_savoy@ti.com wrote:
> > > > >>>> From: Pavan Savoy <pavan_savoy@ti.com>
> > > > >>>>
> > > > >>>> 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@ti.com>
> > > > >>>
> > > > >>> 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@oracle.com>
> > > >
> > > >
> > > > 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.
> > >
> > > Yes, I will work on it, sorry, been swamped with other work lately.
> > >
> > > Don't worry, this patch is in my "to-apply" queue.
> >
> > Any update on this?
> 
> Nope, sorry.  I got sick and combined with vacation requirements from
> the family, and my "real job" and other things lately, I hope to get to
> it by the end of this week.  If not, kick me, I'll have time on a
> trans-continent flight next week that should give me plenty of time to
> do this...

Gzee Thanks, so since you added the patch now, I guess you will be reviewing
The ti-st driver and sketch out plans to move it out of staging on your long trans-Continent flight?


> thanks,
> 
> greg k-h

Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Greg KH]

On Wed, Sep 01, 2010 at 03:53:19AM +0530, Savoy, Pavan wrote:
> 
> Greg,
> Gzee Thanks, so since you added the patch now, I guess you will be
> reviewing The ti-st driver and sketch out plans to move it out of
> staging on your long trans-Continent flight?

Yes, I will try to do that.

thanks,

greg k-h

RE: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Savoy, Pavan]

Greg,
 

> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Tuesday, August 31, 2010 5:43 PM
> To: Savoy, Pavan
> Cc: linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org
> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
> 
> On Wed, Sep 01, 2010 at 03:53:19AM +0530, Savoy, Pavan wrote:
> >
> > Greg,
> > Gzee Thanks, so since you added the patch now, I guess you will be
> > reviewing The ti-st driver and sketch out plans to move it out of
> > staging on your long trans-Continent flight?
> 
> Yes, I will try to do that.
> 

Just checking if you had time to review the driver to be moved out of staging...

> 
> greg k-h

Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Anca Emanuel]

Build error (kernel 2.6.36-rc3-git1):


Building modules, stage 2.
  MODPOST 3056 modules
ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!

RE: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Savoy, Pavan]

Emanuel,

I've recently posted a patch to solve this problem.
Patch @ http://lkml.org/lkml/2010/8/19/238
 
PS: In fact it is strange that you replied to the same thread which is supposed to solve the problem.

Thanks...

> -----Original Message-----
> From: Anca Emanuel [mailto:anca.emanuel@gmail.com]
> Sent: Wednesday, September 08, 2010 1:27 AM
> To: Savoy, Pavan
> Cc: Greg KH; linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org
> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
> 
> Build error (kernel 2.6.36-rc3-git1):
> 
> 
> Building modules, stage 2.
>   MODPOST 3056 modules
> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!

Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Anca Emanuel]

Linus, this patch is not in your tree.

please fix this.

On Wed, Sep 8, 2010 at 5:39 PM, Savoy, Pavan <pavan_savoy@ti.com> wrote:
> Emanuel,
>
> I've recently posted a patch to solve this problem.
> Patch @ http://lkml.org/lkml/2010/8/19/238
>
> PS: In fact it is strange that you replied to the same thread which is supposed to solve the problem.
>
> Thanks...
>
>> -----Original Message-----
>> From: Anca Emanuel [mailto:anca.emanuel@gmail.com]
>> Sent: Wednesday, September 08, 2010 1:27 AM
>> To: Savoy, Pavan
>> Cc: Greg KH; linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org
>> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
>>
>> Build error (kernel 2.6.36-rc3-git1):
>>
>>
>> Building modules, stage 2.
>>   MODPOST 3056 modules
>> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!
>

RE: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Savoy, Pavan]

Emanuel,
 

> -----Original Message-----
> From: Anca Emanuel [mailto:anca.emanuel@gmail.com]
> Sent: Wednesday, September 08, 2010 2:03 PM
> To: Savoy, Pavan
> Cc: Greg KH; linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org; Linus
> Torvalds
> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
> 
> Linus, this patch is not in your tree.
> 
> please fix this.

I don't think it needs to be for now.
It will get merged upon next update from linux-next.

It is currently in linux-next.


> On Wed, Sep 8, 2010 at 5:39 PM, Savoy, Pavan <pavan_savoy@ti.com> wrote:
> > Emanuel,
> >
> > I've recently posted a patch to solve this problem.
> > Patch @ http://lkml.org/lkml/2010/8/19/238
> >
> > PS: In fact it is strange that you replied to the same thread which is
> supposed to solve the problem.
> >
> > Thanks...
> >
> >> -----Original Message-----
> >> From: Anca Emanuel [mailto:anca.emanuel@gmail.com]
> >> Sent: Wednesday, September 08, 2010 1:27 AM
> >> To: Savoy, Pavan
> >> Cc: Greg KH; linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org
> >> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
> >>
> >> Build error (kernel 2.6.36-rc3-git1):
> >>
> >>
> >> Building modules, stage 2.
> >>   MODPOST 3056 modules
> >> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!
> >

Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Anca Emanuel]

> It is currently in linux-next.
[...]
>> >> Build error (kernel 2.6.36-rc3-git1):
>> >>
>> >>
>> >> Building modules, stage 2.
>> >>   MODPOST 3056 modules
>> >> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!

Greg,

you send some pull requests to Linus.
Can you compile the kernel without this patch ?

see here:
http://kernel.ubuntu.com/~kernel-ppa/mainline/v2.6.36-rc5-maverick/0002-DISABLE-ti-st.patch

I know you are overburned. Just get 1 or 2 guys to teach them to help you.

Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Greg KH]

On Tue, Sep 21, 2010 at 08:34:51PM +0300, Anca Emanuel wrote:
> > It is currently in linux-next.
> [...]
> >> >> Build error (kernel 2.6.36-rc3-git1):
> >> >>
> >> >>
> >> >> Building modules, stage 2.
> >> >>   MODPOST 3056 modules
> >> >> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!
> 
> Greg,
> 
> you send some pull requests to Linus.
> Can you compile the kernel without this patch ?

It works here for me, doesn't it for you?

> see here:
> http://kernel.ubuntu.com/~kernel-ppa/mainline/v2.6.36-rc5-maverick/0002-DISABLE-ti-st.patch
> 
> I know you are overburned. Just get 1 or 2 guys to teach them to help you.

I still don't understand, what does that patch have to do with the
staging tree?

thanks,

greg k-h

Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Anca Emanuel]

>> see here:
>> http://kernel.ubuntu.com/~kernel-ppa/mainline/v2.6.36-rc5-maverick/0002-DISABLE-ti-st.patch
>>
>> I know you are overburned. Just get 1 or 2 guys to teach them to help you.
>
> I still don't understand, what does that patch have to do with the
> staging tree?
>
> thanks,
>
> greg k-h
>

In Ubuntu, the ti-st driver have a problem at the building time.

Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Greg KH]

On Tue, Sep 21, 2010 at 09:06:07PM +0300, Anca Emanuel wrote:
> >> see here:
> >> http://kernel.ubuntu.com/~kernel-ppa/mainline/v2.6.36-rc5-maverick/0002-DISABLE-ti-st.patch
> >>
> >> I know you are overburned. Just get 1 or 2 guys to teach them to help you.
> >
> > I still don't understand, what does that patch have to do with the
> > staging tree?
> >
> > thanks,
> >
> > greg k-h
> >
> 
> In Ubuntu, the ti-st driver have a problem at the building time.

Ok, then in Ubuntu, don't enable it in the configuration file, no need
to comment out the whole directory :)

Is there a specific patch that fixes this issue that is not around?  Or
is it already in my staging-next tree and you feel it should be added to
the final .36 release?

If so, which one is it?

thanks,

greg k-h

RE: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Savoy, Pavan]

Emanuel,
 

> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Tuesday, September 21, 2010 1:13 PM
> To: Anca Emanuel
> Cc: Savoy, Pavan; linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org;
> Linus Torvalds
> Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
> 
> On Tue, Sep 21, 2010 at 09:06:07PM +0300, Anca Emanuel wrote:
> > >> see here:
> > >> http://kernel.ubuntu.com/~kernel-ppa/mainline/v2.6.36-rc5-maverick/0002-
> DISABLE-ti-st.patch
> > >>
> > >> I know you are overburned. Just get 1 or 2 guys to teach them to help
> you.
> > >
> > > I still don't understand, what does that patch have to do with the
> > > staging tree?
> > >
> > > thanks,
> > >
> > > greg k-h
> > >
> >
> > In Ubuntu, the ti-st driver have a problem at the building time.
> 
> Ok, then in Ubuntu, don't enable it in the configuration file, no need
> to comment out the whole directory :)
> 
> Is there a specific patch that fixes this issue that is not around?  Or
> is it already in my staging-next tree and you feel it should be added to
> the final .36 release?
> 
> If so, which one is it?

This patch on the ti-st/ inside staging/ fixes the problem.
It has made it to the linux-next,
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=dbd3a8709560365ff9b1e5eca263f608877a8a89


> thanks,
> 
> greg k-h

Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Anca Emanuel]

> Is there a specific patch that fixes this issue that is not around?  Or
> is it already in my staging-next tree and you feel it should be added to
> the final .36 release?
>
> If so, which one is it?
>
> thanks,
>
> greg k-h
>

for the build problem, include it in 2.6.36
no need for more delay.

Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device

[Greg KH]

On Tue, Sep 21, 2010 at 11:48:58PM +0530, Savoy, Pavan wrote:
> 
> Emanuel,
>  
> > If so, which one is it?
> 
> This patch on the ti-st/ inside staging/ fixes the problem.
> It has made it to the linux-next,
> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=dbd3a8709560365ff9b1e5eca263f608877a8a89

I'll add it to the queue to go to Linus earlier if I get the chance.  If
not, it's not a big deal we can add it to the .36-stable tree.

thanks,

greg k-h