Re: [PATCH] drivers:staging: sources for ST core
From: Pavan Savoy
Date: Thu Apr 01 2010 - 13:21:23 EST
--- On Thu, 1/4/10, Alan Cox <alan@xxxxxxxxxxxxxxxxxxx> wrote:
> From: Alan Cox <alan@xxxxxxxxxxxxxxxxxxx>
> Subject: Re: [PATCH] drivers:staging: sources for ST core
> To: pavan_savoy@xxxxxx
> Cc: "Greg KH" <gregkh@xxxxxxx>, "Marcel Holtmann" <marcel@xxxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx
> Date: Thursday, 1 April, 2010, 2:50 PM
> > +/*
> > + * function to return whether the firmware response
> was proper
> > + * in case of error don't complete so that waiting
> for proper
> > + * response times out
> > + */
> > +void validate_firmware_response(struct sk_buff *skb)
> > +{
> > + if (unlikely(skb->data[5] !=
> 0)) {
> > + pr_err("no
> proper response during fw download");
> > + pr_err("data6
> %x", skb->data[5]);
>
> In this driver you do know the device so you need to be
> using dev_ and
> passing around dev (or something that gives you dev).
>
> > +static int kim_probe(struct platform_device *pdev)
> > +{
> > + long status;
> > + long proto;
> > + long *gpios =
> pdev->dev.platform_data;
> > +
> > + status =
> st_core_init(&kim_gdata->core_data);
>
> I would expect any truely global data to be configured in
> the module init
> and then device specific data you want to do something like
> this
>
> kim_data = kzalloc(sizeof(something),
> GFP_KERNEL);
>
> ..
>
> kim_data_init(&pdev->dev,
> kim_data);
> dev_set_drvdata(&pdev->dev,
> kim_data);
>
> Elsewhere you can now do
>
> kim_data =
> dev_get_drvdata(&pdev->dev);
>
> to get it back
There are 2 sets of data structures here (after removing the un-necessary 3rd one),
1. st_gdata - which I would want to tie to tty
2. kim_gdata - which I "would" like to tie to the pdev.
Now the problem being, I have reference of st_gdata in kim_gdata, and there are about 4 functions in the st_core where I would need the st_gdata, as in,
EXPORTED symbol st_register - because I need to add in entries as to who registered,
+long st_register(struct st_proto_s *new_proto)
+{
+ struct st_data_s *st_gdata;
+ long err = ST_SUCCESS;
+ unsigned long flags = 0;
+
+ st_kim_ref(&st_gdata);
+ 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) {
+ pr_err("gdata/new_proto/recv or reg_complete_cb not ready");
+ return ST_ERR_FAILURE;
+ }
Also in st_unregister and st_write for similar purposes,
But I also need it in tty_open, to link it to the disc_data,
+static int st_tty_open(struct tty_struct *tty)
+{
+ int err = ST_SUCCESS;
+ struct st_data_s *st_gdata;
+ pr_info("%s ", __func__);
+
+ st_kim_ref(&st_gdata);
+ st_gdata->tty = tty;
+ tty->disc_data = st_gdata;
So, shouldn't some function like st_kim_ref be enough ?
+void st_kim_ref(struct st_data_s **core_data)
+{
+ *core_data = kim_gdata->core_data;
+}
So Now st_gdata is tied to tty, and kim_gdata being purely global, as in only 1 platform device of such kind can exist.
Suppose 2 platform devices want to exist - then who's EXPORT of st_register is considered ?
And If bluetooth or FM wants to use this transport then, how would it tell onto which platform device it wants to attach to ?
Why should I tie kim_gdata to a pdev ?
> --
> 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/
>
The INTERNET now has a personality. YOURS! See your Yahoo! Homepage. http://in.yahoo.com/
--
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/