From: Alan Cox on
On Tue, 4 May 2010 21:55:43 +0530 (IST)
Pavan Savoy <pavan_savoy(a)ti.com> wrote:

> Greg, Alan,
>
> Just to complete the circle on N_TI_WL, find below the GPS driver which makes use of the shared transport line discipline.
>
> This driver provides a TTY line character device to application/middle-ware running on host, as if the device is directly connected over UART to a GPS chip.

This doesn't appear to be a tty device ?

> Almost all actions that can be done on a /dev/ttySx can be done on this /dev/tigps device.

Hardly true. A tty driver has a very precisely defined set of behaviours
and a lot of ioctls and interfaces your driver doesn't. Our gps
interfaces are only tty drivers because historically they were plugged
into serial ports so I'm not sure the 'not a tty' bit actually matters.

Codewise its the same as all the rest - only one instance possible and
poking around in globals with no visible or documented locking.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Savoy, Pavan on
Alan,


----------------
Thanks & Regards,
Pavan Savoy | x0099669

> -----Original Message-----
> From: Alan Cox [mailto:alan(a)lxorguk.ukuu.org.uk]
> Sent: Wednesday, May 05, 2010 6:12 AM
> To: Savoy, Pavan
> Cc: greg; linux-kernel(a)vger.kernel.org
> Subject: Re: [PATCH] drivers: staging: GPS protocol driver for wl128x
>
> On Tue, 4 May 2010 21:55:43 +0530 (IST)
> Pavan Savoy <pavan_savoy(a)ti.com> wrote:
>
> > Greg, Alan,
> >
> > Just to complete the circle on N_TI_WL, find below the GPS driver which makes use of the shared transport
> line discipline.
> >
> > This driver provides a TTY line character device to application/middle-ware running on host, as if the
> device is directly connected over UART to a GPS chip.
>
> This doesn't appear to be a tty device ?

A typo here, I meant "TTY like" character device, and it not certainly a character device.

>
> > Almost all actions that can be done on a /dev/ttySx can be done on this /dev/tigps device.
>
> Hardly true. A tty driver has a very precisely defined set of behaviours
> and a lot of ioctls and interfaces your driver doesn't. Our gps
> interfaces are only tty drivers because historically they were plugged
> into serial ports so I'm not sure the 'not a tty' bit actually matters.

Yes. I agree, and hence mentioned almost all.

> Codewise its the same as all the rest - only one instance possible and
> poking around in globals with no visible or documented locking.

Yes, and this is the reason, I posted this patch.
BT and GPS had to communicate over a single UART, and this is the reason the N_TI_WL line discipline exists.
With this sort of architecture, how can I accommodate multi-device support? To avoid this single device limits?

Also, what is that you are exactly looking for regarding locking.
Please suggest.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Savoy, Pavan on

Alan,

> -----Original Message-----
> From: Savoy, Pavan
> Sent: Wednesday, May 05, 2010 11:04 AM
> To: 'Alan Cox'
> Cc: greg; linux-kernel(a)vger.kernel.org
> Subject: RE: [PATCH] drivers: staging: GPS protocol driver for wl128x
>
> Alan,
>
>
> ----------------
> Thanks & Regards,
> Pavan Savoy | x0099669
>
> > -----Original Message-----
> > From: Alan Cox [mailto:alan(a)lxorguk.ukuu.org.uk]
> > Sent: Wednesday, May 05, 2010 6:12 AM
> > To: Savoy, Pavan
> > Cc: greg; linux-kernel(a)vger.kernel.org
> > Subject: Re: [PATCH] drivers: staging: GPS protocol driver for wl128x
> >
> > On Tue, 4 May 2010 21:55:43 +0530 (IST)
> > Pavan Savoy <pavan_savoy(a)ti.com> wrote:
> >
> > > Greg, Alan,
> > >
> > > Just to complete the circle on N_TI_WL, find below the GPS driver which makes use of the shared
> transport
> > line discipline.
> > >
> > > This driver provides a TTY line character device to application/middle-ware running on host, as if the
> > device is directly connected over UART to a GPS chip.
> >
> > This doesn't appear to be a tty device ?
>
> A typo here, I meant "TTY like" character device, and it not certainly a character device.
>
> >
> > > Almost all actions that can be done on a /dev/ttySx can be done on this /dev/tigps device.
> >
> > Hardly true. A tty driver has a very precisely defined set of behaviours
> > and a lot of ioctls and interfaces your driver doesn't. Our gps
> > interfaces are only tty drivers because historically they were plugged
> > into serial ports so I'm not sure the 'not a tty' bit actually matters.
>
> Yes. I agree, and hence mentioned almost all.
>
> > Codewise its the same as all the rest - only one instance possible and
> > poking around in globals with no visible or documented locking.

To support this multiple device thingy and avoid the single device limit, I plan to do something like this,

The ST driver would be platform_device - as it is already. ST's probe would do the ldisc registration, and also do the dev_set_drvdata of all the internal data (Tx queues, locks, list of protocols) on this device.

Apart from this the BT, FM and GPS would also further be platform devices, and then,
In the dev.platform_data of each of these BT, FM and GPS devices, I will enter the "parent ST device", these protocols want to attach themselves to.

And in probe of the driver for each of these BT, FM and GPS devices, I will do the same "st_register" where in now I can access the ST related data by retrieving the internal using the parent device.

Examples:
Here are the 2 ST platform devices,
struct platform_device st_device_0 = {
.name = "ST_DEV_0",
.id = -1,
.dev.platform_data = &array,
.dev.release = any_device_release,
};
struct platform_device st_device_1 = {
.name = "ST_DEV_1",
.id = -1,
.dev.platform_data = &array,
.dev.release = any_device_release,
};

BT attaching itself to ST_DEV_0,
static struct protocol_platform_data bt_data = {
.parent_dev = &st_device_0,
.name = "BT",
};

static struct platform_device bt_device = {
.name = "BT",
.id = -1,
.dev.platform_data = &bt_data,
.dev.release = bt_device_release,
};

And FM attaching itself to - ST_DEV_1,
static struct protocol_platform_data fm_data = {
.parent_dev = &st_device_1,
.name = "FM",
};
static struct platform_device fm_device = {
.name = "FM",
.id = -1,
.dev.platform_data = &fm_data,
.dev.release = fm_device_release,
};

My "ST_REGISTER" would now look like,
int st_register(struct protocol_platform_data *data)
{
struct st_data_s *st_data;
struct platform_device *parent = data->parent_dev;

st_data = dev_get_drvdata(&parent->dev);

printk("%s registering into %s\n", data->name, parent->name);

printk("data got is %s\n", st_data->name);
st_data->name = "Changed Name";
return 0;
}
EXPORT_SYMBOL(st_register);


However, I would need to have a ST platform driver for each of my such ST platform devices (ST_DEV_0, ST_DEV_1).

It does seem a big-re-write of the probing/registration logic..
So, How does this sound?

Please suggest.


> Yes, and this is the reason, I posted this patch.
> BT and GPS had to communicate over a single UART, and this is the reason the N_TI_WL line discipline exists.
> With this sort of architecture, how can I accommodate multi-device support? To avoid this single device
> limits?
>
> Also, what is that you are exactly looking for regarding locking.
> Please suggest.
>
> Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Pavan Savoy on


--- On Fri, 7/5/10, Savoy, Pavan <pavan_savoy(a)ti.com> wrote:

> From: Savoy, Pavan <pavan_savoy(a)ti.com>
> Subject: RE: [PATCH] drivers: staging: GPS protocol driver for wl128x
> To: "Alan Cox" <alan(a)lxorguk.ukuu.org.uk>
> Cc: "greg" <gregkh(a)suse.de>, "linux-kernel(a)vger.kernel.org" <linux-kernel(a)vger.kernel.org>, "Menon, Nishanth" <nm(a)ti.com>
> Date: Friday, 7 May, 2010, 2:00 AM
>
> Alan,
>
> > -----Original Message-----
> > From: Savoy, Pavan
> > Sent: Wednesday, May 05, 2010 11:04 AM
> > To: 'Alan Cox'
> > Cc: greg; linux-kernel(a)vger.kernel.org
> > Subject: RE: [PATCH] drivers: staging: GPS protocol
> driver for wl128x
> >
> > Alan,
> >
> >
> > ----------------
> > Thanks & Regards,
> > Pavan Savoy | x0099669
> >
> > > -----Original Message-----
> > > From: Alan Cox [mailto:alan(a)lxorguk.ukuu.org.uk]
> > > Sent: Wednesday, May 05, 2010 6:12 AM
> > > To: Savoy, Pavan
> > > Cc: greg; linux-kernel(a)vger.kernel.org
> > > Subject: Re: [PATCH] drivers: staging: GPS
> protocol driver for wl128x
> > >
> > > On Tue, 4 May 2010 21:55:43 +0530 (IST)
> > > Pavan Savoy <pavan_savoy(a)ti.com>
> wrote:
> > >
> > > > Greg, Alan,
> > > >
> > > > Just to complete the circle on N_TI_WL, find
> below the GPS driver which makes use of the shared
> > transport
> > > line discipline.
> > > >
> > > > This driver provides a TTY line character
> device to application/middle-ware running on host, as if
> the
> > > device is directly connected over UART to a GPS
> chip.
> > >
> > > This doesn't appear to be a tty device ?
> >
> > A typo here, I meant "TTY like" character device, and
> it not certainly a character device.
> >
> > >
> > > > Almost all actions that can be done on a
> /dev/ttySx can be done on this /dev/tigps device.
> > >
> > > Hardly true. A tty driver has a very precisely
> defined set of behaviours
> > > and a lot of ioctls and interfaces your driver
> doesn't. Our gps
> > > interfaces are only tty drivers because
> historically they were plugged
> > > into serial ports so I'm not sure the 'not a tty'
> bit actually matters.
> >
> > Yes. I agree, and hence mentioned almost all.
> >
> > > Codewise its the same as all the rest - only one
> instance possible and
> > > poking around in globals with no visible or
> documented locking.
>
> To support this multiple device thingy and avoid the single
> device limit, I plan to do something like this,
>
> The ST driver would be platform_device - as it is already.
> ST's probe would do the ldisc registration, and also do the
> dev_set_drvdata of all the internal data (Tx queues, locks,
> list of protocols) on this device.
>
> Apart from this the BT, FM and GPS would also further be
> platform devices, and then,
> In the dev.platform_data of each of these BT, FM and GPS
> devices, I will enter the "parent ST device", these
> protocols want to attach themselves to.
>
> And in probe of the driver for each of these BT, FM and GPS
> devices, I will do the same "st_register" where in now I can
> access the ST related data by retrieving the internal using
> the parent device.
>
> Examples:
> Here are the 2 ST platform devices,
> struct platform_device st_device_0 = {
> � � � � .name = "ST_DEV_0",
> � � � � .id = -1,
> � � � � .dev.platform_data =
> &array,
> � � � � .dev.release =
> any_device_release,
> };
> struct platform_device st_device_1 = {
> � � � � .name = "ST_DEV_1",
> � � � � .id = -1,
> � � � � .dev.platform_data =
> &array,
> � � � � .dev.release =
> any_device_release,
> };
>
> BT attaching itself to ST_DEV_0,
> static struct protocol_platform_data bt_data = {
> � � � � .parent_dev =
> &st_device_0,
> � � � � .name = "BT",
> };
>
> static struct platform_device bt_device = {
> � � � � .name = "BT",
> � � � � .id = -1,
> � � � � .dev.platform_data =
> &bt_data,
> � � � � .dev.release =
> bt_device_release,
> };
>
> And FM attaching itself to - ST_DEV_1,
> static struct protocol_platform_data fm_data = {
> � � � � .parent_dev =
> &st_device_1,
> � � � � .name = "FM",
> };
> static struct platform_device fm_device = {
> � � � � .name = "FM",
> � � � � .id = -1,
> � � � � .dev.platform_data =
> &fm_data,
> � � � � .dev.release =
> fm_device_release,
> };
>
> My "ST_REGISTER" would now look like,
> int st_register(struct protocol_platform_data *data)
> {
> � � � � struct st_data_s *st_data;
> � � � � struct platform_device *parent
> = data->parent_dev;
>
> � � � � st_data =
> dev_get_drvdata(&parent->dev);
>
> � � � � printk("%s registering into
> %s\n", data->name, parent->name);
>
> � � � � printk("data got is %s\n",
> st_data->name);
> � � � � st_data->name = "Changed
> Name";
> � � � � return 0;
> }
> EXPORT_SYMBOL(st_register);
>
>
> However, I would need to have a ST platform driver for each
> of my such ST platform devices (ST_DEV_0, ST_DEV_1).
>
> It does seem a big-re-write of the probing/registration
> logic..
> So, How does this sound?
>
> Please suggest.

Just to add, I would have 1 more problem to address, How can I link up my tty->disc_data with my platform-data ?
On registering ldisc_ops I would have liked to have an _private data sent in during tty_open/close/ioctl calls - which currently ldisc_ops doesn't support.

>
> > Yes, and this is the reason, I posted this patch.
> > BT and GPS had to communicate over a single UART, and
> this is the reason the N_TI_WL line discipline exists.
> > With this sort of architecture, how can I accommodate
> multi-device support? To avoid this single device
> > limits?
> >
> > Also, what is that you are exactly looking for
> regarding locking.
> > Please suggest.
> >
> > Thanks.
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> the body of a message to majordomo(a)vger.kernel.org
> 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(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/