Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pali RohÃr]

On Tuesday 22 January 2013 17:17:21 Felipe Balbi wrote:
> Hi,
> 
> On Tue, Jan 22, 2013 at 12:03:09PM +0100, Pali RohÃr wrote:
> > On Monday 21 January 2013 09:05:06 Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Sun, Jan 20, 2013 at 11:17:31AM +0100, Pali RohÃr wrote:
> > > > On Sunday 20 January 2013 10:25:37 Felipe Balbi wrote:
> > > > > On Sun, Jan 20, 2013 at 03:58:13AM +0100, Pali RohÃr 
wrote:
> > > > > > Signed-off-by: Pali RohÃr <pali.rohar@xxxxxxxxx>
> > > > > 
> > > > > NAK for two reasons:
> > > > > 
> > > > > a) the original Nokia kernel used a separate
> > > > > g_file_storage gadget to use Mass Storage mode, use
> > > > > that
> > > > > 
> > > > > b) there is no commit log
> > > > 
> > > > Reason why add mass storage mode to g_nokia is to avoid
> > > > switching between g_{file,mass}_storage and g_nokia and
> > > > to have one gadget driver for Nokia N900. It is better
> > > > to have usb network and mass storage mode in one driver
> > > > (and not to unload & load another).
> > > > 
> > > > I tested this patch with 3.8-rc3 kernel on Nokia N900
> > > > and usb network with mass storage mode working without
> > > > problems.
> > > 
> > > Doesn't matter, in this case this is something which nokia
> > > wrote to carry on their Maemo/MeeGo devices so unless
> > > someone from Nokia says this is how they want to use
> > > nokia.c from now on, I can't simply risk breaking all
> > > other users for your own convenience.
> > 
> > Hello,
> > 
> > you may know that Nokia not working on any linux Maemo/MeeGo
> > systems anymore. And also that Nokia devices needs own
> > patched kernel. Also g_nokia gadget is for Nokia N900 and
> > this device with original Maemo 5 system which was
> > supported by Nokia is locked for patched 2.6.28 kernel.
> > More drivers in 2.6.28 was not upstreamed, so running other
> > kernels will not work without problems. And waiting what
> > Nokia say is now irrelevant, because
> 
> nonsense, many guys (including myself) have n900 booting
> mainline kernel.
> 

Hi,

this sounds good. Do you use some n900 specified patches with 
upstream kernel? There are more drivers which are not present in 
mainline (like bluetooth, camera or ssi).

To reduce divergence I created git repo for n900 patches on 
gitorious https://gitorious.org/linux-n900/linux-n900 where I 
pushing needed patched for n900 hw (now based on 3.8-rc3)

I know only few people which hacking n900 with upstream kernel. 
And all are on freenode #maemo-alternatives channel. Everybody 
with some new upstream kernel is welcome.

Btw, now I have working kernel drivers for charging battery on 
n900, so charging is solved.

> > N900 is at end-of-live cycle and Nokia not doing with linux
> > devices anymore. So I do not understand why current code
> > cannot be extended for more functionality. Because patch is
> > not signed by Nokia?
> 
> If I let your change in, you could be breaking the folks who
> are still using n900 as their daily device. Remember that
> Mass Storage access to the media, prevents phone from using
> mass storage too and the way Maemo 5 was done is that it will
> unload mass storage when that's not being used.
> 

This is not truth. I commited more patched to CSSU ke-recv maemo 
daemon (which handle pcsuite/mass storage mode and mounting 
MyDocs and SD card) and I know how it working. That daemon do not 
unload g_file_storage driver when mass storage mode is 
deactivated. Instead it will call "echo > /sys/something/lun" 
which cause to disconnect mmc device from gadget. This working 
fine, because gadget is loaded with removeable param. And because 
there are some other problems with BME and maemo kernel, you need 
to have loaded some gadget for proper wall charger detection in 
BME. And maemo upstart daemon loading g_file_storage at boot time 
with none mmc device. So this is not problem.

If you can see at my patch, it will create mass storage gadget 
with removable param too. So block device can be 
attached/deattached from gadget at runtime without problem. Also 
when gadget is loaded nothing is attached.

> > There are more developers which playing with upstream kernel
> > on Nokia N900 and trying to use some modern linux
> > distribution on it. And who using upstream kernels on N900
> > also want some additional functionality which was not in
> > 2.6.28. And having mass storage in g_nokia is usefull.
> 
> it makes no difference.
> 
> > Also you can see that this patch simply adding new composite
> > gadget to exisitng driver. Nothing is removed, so original
> > code is compatible with these changes. If somebody still
> > want (for
> 
> you could be breaking phone's own access to the memory card.
> 

No, because by default is nothing attached to gadget.

> > some reason) to switching between g_nokia and g_file_storage
> > (ops, it was renamed to g_mass_storage, so now it is broken
> > on old Nokia systems...) it is still possible.
> 
> no it's not, you can anyways make a symbolic link. Besides,
> before removing g_file_storage we waited quite some time.
> 
> > And when I looked into nokia 2.6.28 kernel, they also
> > patched g_file_storage, so I think it is incompatible with
> > upstream too.
> 
> it's patched only for strings.
> 

Not exactly, when I looked at diff there was some other code and 
comments with "hacks". But changes for this are irrelevant now...

> > So why to care about current API implementation in upstream
> > kernel (do not allow to add new functionality) which is
> > incompatible with Nokia patched kernel?
> 
> Because I don't think g_nokia needs mass storage and because I
> want to remove all gadget drivers from kernel (keep only
> function drivers).

Ok, if you remove all gadget from kernel, this will break 
compatibility for sure.

And what will be equivalent of g_nokia? Will there be some 
"universal" composite gadget (which will have support for serial 
port, mass storage and usb network)?

-- 
Pali RohÃr
pali.rohar@xxxxxxxxx

Attachment: signature.asc
Description: This is a digitally signed message part.