[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4C6D6A9A.3010409@oracle.com>
Date: Thu, 19 Aug 2010 10:32:10 -0700
From: Randy Dunlap <randy.dunlap@...cle.com>
To: pavan_savoy@...com
CC: gregkh@...e.de, alan@...rguk.ukuu.org.uk,
linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org,
naveen_jain@...com
Subject: Re: [PATCH] drivers:staging:ti-st: remove st_get_plat_device
On 08/19/10 11:08, pavan_savoy@...com wrote:
> From: Pavan Savoy <pavan_savoy@...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@...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 ***
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists