Re: How to replace control code in gdm72xx?

2014-07-25 Thread Michalis Pappas
On 07/17/2014 01:59 AM, Ben Chan wrote:
 
 From what I read from the user-space code, the custom ioctl is to read
 the following information from the driver:
 
 enum {
 SIOC_DATA_FSM,   // Device/Connection State
 SIOC_DATA_NETLIST,   // Network list
 SIOC_DATA_CONNNSP, // Connected NSP
 SIOC_DATA_CONNCOMP,// Connection completion info
 SIOC_DATA_PROFILEID,// WiMAX profile ID
 SIOC_DATA_END
 };
 

Sorry for the late reply, I was away. 

I had a look at the library's source to clear things out a bit. From what I see 
the driver is tightly coupled -actually dependent- to the library, as all 
control operations are performed by the latter. So without the library, the 
driver is pretty much of no use.

The way control works is that the driver acts as a middleman between the 
library and the hci. All operations are initiated and handled by the library, 
through the netlink channel. So the information mentioned above, eg 
SIOC_DATA_NETLIST, are requested by the library, by sending the appropriate 
command to the driver via netlink. The driver will forward the request to the 
hci and forward the response back to userspace.

Here is where the ioctl part interface into play. The ioctl interface supports 
two commands: SIOCS_DATA and SIOCG_DATA (set data / get data). The above can 
store and retrieve data from the driver, which in turn keeps this information 
in the nic.sdk_data array. This array maintains one element for each one of the 
SIOC_DATA_* mentioned above.

So once received and processed, the library will store the received data (eg 
the network list) in the driver's memory via the ioctl, just to retrieve it 
again later(!) With the exception of SIOC_FSM_XXX, the driver does not inspect 
or process the contents of these objects by any means (see gdm_wimax_ioctl() in 
gdm_wimax.c).

Now, I have no clue why it was written this way, but as per its current 
status all this might as well (and probably should) be stored locally, in 
userspace memory. So the ioctl part can be completely removed, and the driver 
will then be in conformance with the interface defined by the wimax stack, in 
wimax.h.

 
 The driver has a corresponding user-space library, so I'd like to
 figure out a way to maintain backward compatibility if possible. But
 I'm happy to help make minor changes to  the user-space library in
 case we need to modify the ioctl / netlink part of the driver (caveat:
 I'm not the author of the driver or the user-space library).
 

Why maintain backwards compatibility? The only part to change is the
communication layer between the library and the driver. The applications
using the library won't notice any difference.

IMO the right thing to do is to push whatever changes made to the library 
upstream. Which brings us to the question of who is the library's maintainer 
and where is it officially hosted?

If the answers to the above are none and nowhere, then we have a bigger 
problem, as the updated driver will appear to be broken to anyone who hasn't 
been updated with the latest version of the library.

In conclusion, I am happy to fix the driver and even write a patch for the 
corresponding userspace parts, but only as long as:

* You are willing to help me with testing, as I do not own the hardware.

* My work will not be waisted. What I mean by that is, if the library has been 
distributed only to a few parties worldwide, then IMO we'd better delete the 
driver and let it be distributed along with the library, since they are 
interdependent anyway.

In any other case, I'd rather move on to some other driver that does not depend 
on third party stuff :)

Thanks,

Michalis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


How to replace control code in gdm72xx?

2014-07-16 Thread Michalis Pappas
Hi,

I'm currently working on bringing the gdm72xx WiMAX driver out of
staging. The driver currently uses two control channels:

1. The SIOCDEVPRIVATE ioctl to send and receive state messages

2. A customly defined netlink protocol for passing messages verbatim to
the device controller

AFAIK both of the above are deprecated, so I considered switching to
the interface defined in wimax.h, which defines a communication protocol
over generic netlink that replaces (2) nicely. However it is not
compatible with (1), as:

* Except from the device status, the gdm72xx driver uses two more types
of messages (connection and OMA status), which is not supported
by wimax.h.

* The gdm driver needs to be able to receive status messages from
userspace, which is not supported by wimax.h either.

I therefore consider using the wimax stack as defined in wimax.h for the
netlink part, but replacing the ioctl with a file under /sys/class/net/wm0/

My questions are whether the above location would be the right place for
that file, and, more importantly, whether this is a generally a valid
approach and not just an ugly workaround.

Thanks,

Michalis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: How to replace control code in gdm72xx?

2014-07-16 Thread Greg KH
On Wed, Jul 16, 2014 at 09:24:28PM +0100, Michalis Pappas wrote:
 Hi,
 
 I'm currently working on bringing the gdm72xx WiMAX driver out of
 staging. The driver currently uses two control channels:
 
 1. The SIOCDEVPRIVATE ioctl to send and receive state messages
 
 2. A customly defined netlink protocol for passing messages verbatim to
 the device controller
 
 AFAIK both of the above are deprecated, so I considered switching to
 the interface defined in wimax.h, which defines a communication protocol
 over generic netlink that replaces (2) nicely. However it is not
 compatible with (1), as:
 
 * Except from the device status, the gdm72xx driver uses two more types
 of messages (connection and OMA status), which is not supported
 by wimax.h.
 
 * The gdm driver needs to be able to receive status messages from
 userspace, which is not supported by wimax.h either.

What type of status messages are needed to be sent to the driver?

 I therefore consider using the wimax stack as defined in wimax.h for the
 netlink part, but replacing the ioctl with a file under /sys/class/net/wm0/

Is anyone still working on wimax to even object to add new functions
like this?  :)

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: How to replace control code in gdm72xx?

2014-07-16 Thread Ben Chan
On Wed, Jul 16, 2014 at 1:40 PM, Greg KH g...@kroah.com wrote:

 On Wed, Jul 16, 2014 at 09:24:28PM +0100, Michalis Pappas wrote:
  Hi,
 
  I'm currently working on bringing the gdm72xx WiMAX driver out of
  staging. The driver currently uses two control channels:
 
  1. The SIOCDEVPRIVATE ioctl to send and receive state messages
 
  2. A customly defined netlink protocol for passing messages verbatim to
  the device controller
 
  AFAIK both of the above are deprecated, so I considered switching to
  the interface defined in wimax.h, which defines a communication protocol
  over generic netlink that replaces (2) nicely. However it is not
  compatible with (1), as:
 
  * Except from the device status, the gdm72xx driver uses two more types
  of messages (connection and OMA status), which is not supported
  by wimax.h.
 
  * The gdm driver needs to be able to receive status messages from
  userspace, which is not supported by wimax.h either.

 What type of status messages are needed to be sent to the driver?


From what I read from the user-space code, the custom ioctl is to read
the following information from the driver:

enum {
SIOC_DATA_FSM,   // Device/Connection State
SIOC_DATA_NETLIST,   // Network list
SIOC_DATA_CONNNSP, // Connected NSP
SIOC_DATA_CONNCOMP,// Connection completion info
SIOC_DATA_PROFILEID,// WiMAX profile ID
SIOC_DATA_END
};

Reference:

https://chromium.googlesource.com/chromiumos/third_party/gdmwimax/+/master/sdk/io.c
https://chromium.googlesource.com/chromiumos/third_party/gdmwimax/+/master/sdk/sdk.c
https://chromium.googlesource.com/chromiumos/third_party/gdmwimax/+/master/sdk/wm_ioctl.h



  I therefore consider using the wimax stack as defined in wimax.h for the
  netlink part, but replacing the ioctl with a file under /sys/class/net/wm0/

 Is anyone still working on wimax to even object to add new functions
 like this?  :)


The driver has a corresponding user-space library, so I'd like to
figure out a way to maintain backward compatibility if possible. But
I'm happy to help make minor changes to  the user-space library in
case we need to modify the ioctl / netlink part of the driver (caveat:
I'm not the author of the driver or the user-space library).

 thanks,

 greg k-h
 ___
 devel mailing list
 de...@linuxdriverproject.org
 http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel