[PATCH 1/2] usb: change the dependency chain for LPC32XX and ISP1301

[PATCH 1/2] usb: change the dependency chain for LPC32XX and ISP1301

[Roland Stigge]

Hi,

On 22/08/12 21:49, Alan Stern wrote:
>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>> index c3f619b..cac3ee2 100644
>> --- a/drivers/usb/host/Kconfig
>> +++ b/drivers/usb/host/Kconfig
>> @@ -292,7 +292,6 @@ config USB_OHCI_HCD
>>  	depends on USB && USB_ARCH_HAS_OHCI
>>  	select ISP1301_OMAP if MACH_OMAP_H2 || MACH_OMAP_H3
>>  	select USB_OTG_UTILS if ARCH_OMAP
>> -	select USB_ISP1301 if ARCH_LPC32XX || ARCH_PNX4008
> 
> You want to avoid selecting USB_ISP1301, right?

The ultimate goal is just to prevent the compile error that currently
occurs when I2C isn't selected on LPC32XX but UDC is. It's an invalid
config combination, not easy to prevent by current Kconfig mechanism.

>> @@ -412,6 +411,15 @@ config USB_CNS3XXX_OHCI
>>  	  Enable support for the CNS3XXX SOC's on-chip OHCI controller.
>>  	  It is needed for low-speed USB 1.0 device support.
>>  
>> +config USB_OHCI_LPC32XX_PNX4008
>> +	bool "LPC32XX or PNX4008 OHCI Module"
>> +	depends on USB_OHCI_HCD
>> +	depends on USB_ISP1301
>> +	---help---
>> +	  Enable support for the LPC32XX or PNX4008 SOC's on-chip
>> +	  OHCI controller.
>> +	  It is needed for low-speed USB 1.0 device support.
> 
> Instead you introduce a new symbol to control whether or not ohci-nxp.c
> gets compiled.  The new symbol depends on USB_ISP1301 but not on
> ARCH_LPC32XX or ARCH_PNX4008, which means in theory it could be defined
> even under a totally different arch.

This issue could be resolved in a different way: Have you followed the
discussion at linux-arm-kernel at lists.infradead.org on 2012-08-20,
Subject "i2c: pnx: Fix bit definitions"? If nothing unexpected turns
out, PNX4008 should probably be removed altogether. (Practically
abandoned arch for 6 years.) Easily possible for v3.7.

Then, the resulting solution is much simpler and Sebastian's patch obsolete.

Thanks,

Roland

[PATCH 1/2] usb: change the dependency chain for LPC32XX and ISP1301

[Sebastian Andrzej Siewior]

On 08/23/2012 12:16 AM, Roland Stigge wrote:
> This issue could be resolved in a different way: Have you followed the
> discussion at linux-arm-kernel at lists.infradead.org on 2012-08-20,
> Subject "i2c: pnx: Fix bit definitions"? If nothing unexpected turns
> out, PNX4008 should probably be removed altogether. (Practically
> abandoned arch for 6 years.) Easily possible for v3.7.

Ach. I assumed that is being merged. The PNX arch does not define
USB_ARCH_HAS_OHCI so I goes that you can't select it there?

> Then, the resulting solution is much simpler and Sebastian's patch obsolete.

That would be what?

>
> Thanks,
>
> Roland

Sebastian

[PATCH 1/2] usb: change the dependency chain for LPC32XX and ISP1301

[Roland Stigge]

On 08/23/2012 09:21 AM, Sebastian Andrzej Siewior wrote:
> On 08/23/2012 12:16 AM, Roland Stigge wrote:
>> This issue could be resolved in a different way: Have you followed the
>> discussion at linux-arm-kernel at lists.infradead.org on 2012-08-20,
>> Subject "i2c: pnx: Fix bit definitions"? If nothing unexpected turns
>> out, PNX4008 should probably be removed altogether. (Practically
>> abandoned arch for 6 years.) Easily possible for v3.7.
> 
> Ach. I assumed that is being merged. The PNX arch does not define
> USB_ARCH_HAS_OHCI so I goes that you can't select it there?

Yes, this issue will be gone also.

>> Then, the resulting solution is much simpler and Sebastian's patch
>> obsolete.
> 
> That would be what?

Well, your idea won't be obsolete completely. :-) But much simpler
without all the "PNX4008 || LPC32XX" stuff. Following Alan's proposal,
I'd prefer not introducing new symbols, and much of the "PNX4008 ||
LPC32XX" logic won't be necessary anymore.

For the PNX4008 removal - should I already post a patch to build the
respective changes on top of, without PNX4008 references?

Thanks,

Roland

[PATCH 1/2] usb: change the dependency chain for LPC32XX and ISP1301

[Sebastian Andrzej Siewior]

On 08/23/2012 10:18 AM, Roland Stigge wrote:
> On 08/23/2012 09:21 AM, Sebastian Andrzej Siewior wrote:
>> On 08/23/2012 12:16 AM, Roland Stigge wrote:
>>> This issue could be resolved in a different way: Have you followed the
>>> discussion at linux-arm-kernel at lists.infradead.org on 2012-08-20,
>>> Subject "i2c: pnx: Fix bit definitions"? If nothing unexpected turns
>>> out, PNX4008 should probably be removed altogether. (Practically
>>> abandoned arch for 6 years.) Easily possible for v3.7.
>>
>> Ach. I assumed that is being merged. The PNX arch does not define
>> USB_ARCH_HAS_OHCI so I goes that you can't select it there?
>
> Yes, this issue will be gone also.
>
>>> Then, the resulting solution is much simpler and Sebastian's patch
>>> obsolete.
>>
>> That would be what?
>
> Well, your idea won't be obsolete completely. :-) But much simpler
> without all the "PNX4008 || LPC32XX" stuff. Following Alan's proposal,
> I'd prefer not introducing new symbols, and much of the "PNX4008 ||
> LPC32XX" logic won't be necessary anymore.
>
> For the PNX4008 removal - should I already post a patch to build the
> respective changes on top of, without PNX4008 references?

If nobody cares about PNX4008 and nobody tested if it actually works in
the OHCI mode I don't see the point in keeping it around especially if
there are rumors to remove the arch.

It would be nice, if you could manage to compile this thingy without
I2C. In the end one could attach something different than isp1301, no?
So I think, in the perfect world you would probe for your phy and
notice that it is not there and print a message: "Dude! Select isp1301
for OHCI, aight?!"

>
> Thanks,
>
> Roland

Sebastian

[PATCH 1/2] usb: change the dependency chain for LPC32XX and ISP1301

[Roland Stigge]

On 08/23/2012 10:29 AM, Sebastian Andrzej Siewior wrote:
>> For the PNX4008 removal - should I already post a patch to build the
>> respective changes on top of, without PNX4008 references?
> 
> If nobody cares about PNX4008 and nobody tested if it actually works in
> the OHCI mode I don't see the point in keeping it around especially if
> there are rumors to remove the arch.
> 
> It would be nice, if you could manage to compile this thingy without
> I2C. In the end one could attach something different than isp1301, no?
> So I think, in the perfect world you would probe for your phy and
> notice that it is not there and print a message: "Dude! Select isp1301
> for OHCI, aight?!"

Currently, we really need I2C for OHCI on LPC32XX, so actually need to
depend on it. Removing the I2C calls in ohci-nxp.c is waiting for the
phy framework so that I don't need to introduce new API in isp1301.c at
this point.

And right, in the ideal world, we have no I2C dependency and can control
a phy in an abstract way, even without depending on ISP1301, which could
be replaced by sth. else (not sure if this will be done/soldered
practically, though).

For now, will post patches for the current I2C dependency and the
PNX4008 removal.

Roland