[PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Arnd Bergmann
The ohci-da8xx driver uses the DA8XX_SYSCFG0_VIRT macro to
access the CFGCHIP2 register for controlling its PHY.

The macro in turn relies on the da8xx_syscfg0_base global
variable. Since the OHCI driver can be a loadable module,
this requires the symbol to be exported from platform code.

Signed-off-by: Arnd Bergmann <[hidden email]>
Cc: Sekhar Nori <[hidden email]>
Cc: Kevin Hilman <[hidden email]>
Cc: [hidden email]
---
 arch/arm/mach-davinci/devices-da8xx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index 0486cdf..4da868a 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -66,6 +66,7 @@
 #define DA850_DMA_MMCSD1_TX EDMA_CTLR_CHAN(1, 29)
 
 void __iomem *da8xx_syscfg0_base;
+EXPORT_SYMBOL_GPL(da8xx_syscfg0_base); /* used by OHCI_HCD */
 void __iomem *da8xx_syscfg1_base;
 
 static struct plat_serial8250_port da8xx_serial0_pdata[] = {
--
1.8.3.2

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Sergei Shtylyov-3
On 03/19/2014 10:29 PM, Arnd Bergmann wrote:

> The ohci-da8xx driver uses the DA8XX_SYSCFG0_VIRT macro to
> access the CFGCHIP2 register for controlling its PHY.

> The macro in turn relies on the da8xx_syscfg0_base global
> variable. Since the OHCI driver can be a loadable module,
> this requires the symbol to be exported from platform code.

> Signed-off-by: Arnd Bergmann <[hidden email]>
> Cc: Sekhar Nori <[hidden email]>
> Cc: Kevin Hilman <[hidden email]>
> Cc: [hidden email]
> ---
>   arch/arm/mach-davinci/devices-da8xx.c | 1 +
>   1 file changed, 1 insertion(+)

> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> index 0486cdf..4da868a 100644
> --- a/arch/arm/mach-davinci/devices-da8xx.c
> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> @@ -66,6 +66,7 @@
>   #define DA850_DMA_MMCSD1_TX EDMA_CTLR_CHAN(1, 29)
>
>   void __iomem *da8xx_syscfg0_base;
> +EXPORT_SYMBOL_GPL(da8xx_syscfg0_base); /* used by OHCI_HCD */

    I have submitted such patch years ago and it was turned down. :-)

WBR, Sergei

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Arnd Bergmann
On Wednesday 19 March 2014 23:53:18 Sergei Shtylyov wrote:

> On 03/19/2014 10:29 PM, Arnd Bergmann wrote:
>
> > The ohci-da8xx driver uses the DA8XX_SYSCFG0_VIRT macro to
> > access the CFGCHIP2 register for controlling its PHY.
>
> > The macro in turn relies on the da8xx_syscfg0_base global
> > variable. Since the OHCI driver can be a loadable module,
> > this requires the symbol to be exported from platform code.
>
> > Signed-off-by: Arnd Bergmann <[hidden email]>
> > Cc: Sekhar Nori <[hidden email]>
> > Cc: Kevin Hilman <[hidden email]>
> > Cc: [hidden email]
> > ---
> >   arch/arm/mach-davinci/devices-da8xx.c | 1 +
> >   1 file changed, 1 insertion(+)
>
> > diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> > index 0486cdf..4da868a 100644
> > --- a/arch/arm/mach-davinci/devices-da8xx.c
> > +++ b/arch/arm/mach-davinci/devices-da8xx.c
> > @@ -66,6 +66,7 @@
> >   #define DA850_DMA_MMCSD1_TX EDMA_CTLR_CHAN(1, 29)
> >
> >   void __iomem *da8xx_syscfg0_base;
> > +EXPORT_SYMBOL_GPL(da8xx_syscfg0_base); /* used by OHCI_HCD */
>
>     I have submitted such patch years ago and it was turned down.
>

The question is whether there is anyone who would do this properly.

Both the OHCI and MUSB drivers use exactly one register (CFGCHIP2)
to control the clock, phy and host/gadget mode switch.

In the modern world, we'd probably want to have a clock driver and
a phy driver for these, based on a syscon driver.

In all honesty I don't see that happening on davinci.

A somewhat better approach would be to export a set of exported
functions to access the one register from the platform, e.g.

u32 da8xx_cfgchip2_get(void);
void da8xx_cfgchip2_set(u32);

That interface would still be a bit ugly, but much better than
what we have today, and easy to implement.

        Arnd
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Sergei Shtylyov-3
On 03/19/2014 11:21 PM, Arnd Bergmann wrote:

>>> The ohci-da8xx driver uses the DA8XX_SYSCFG0_VIRT macro to
>>> access the CFGCHIP2 register for controlling its PHY.

>>> The macro in turn relies on the da8xx_syscfg0_base global
>>> variable. Since the OHCI driver can be a loadable module,
>>> this requires the symbol to be exported from platform code.

>>> Signed-off-by: Arnd Bergmann <[hidden email]>
>>> Cc: Sekhar Nori <[hidden email]>
>>> Cc: Kevin Hilman <[hidden email]>
>>> Cc: [hidden email]
>>> ---
>>>    arch/arm/mach-davinci/devices-da8xx.c | 1 +
>>>    1 file changed, 1 insertion(+)

>>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
>>> index 0486cdf..4da868a 100644
>>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>>> @@ -66,6 +66,7 @@
>>>    #define DA850_DMA_MMCSD1_TX EDMA_CTLR_CHAN(1, 29)
>>>
>>>    void __iomem *da8xx_syscfg0_base;
>>> +EXPORT_SYMBOL_GPL(da8xx_syscfg0_base); /* used by OHCI_HCD */

>>      I have submitted such patch years ago and it was turned down.

    I also had a pleasure of completing implementation of this driver and
submitting it back in 2009 when I was working for MontaVista. :-)

> The question is whether there is anyone who would do this properly.

    Nobody cares, it seems. As you can imagine, I stopped to care enough after
being switched to other projects.

> Both the OHCI and MUSB drivers use exactly one register (CFGCHIP2)

    The idea at the time was to just ioremap() this register but I was not
very eager.
There was no USB PHY driver subsystem yet.

> to control the clock, phy

    Note that it only controls PHY clock (PLL) which could be covered by an
USB PHY driver.

> and host/gadget mode switch.

    The main mode the USB 2.0 PHY should function now is OTG. The host/gadged
modes are forced overrides of the ID pin. Unfortunately, the board code has to
force host mode when host-only driver config is selected (these MUSB's
host/gadget only modes were removed at one point but got reintroduced again).

> In the modern world, we'd probably want to have a clock driver and

    Not sure about the clock driver...

> a phy driver for these,

    Yes, that's what the MUSB maintainer wants too. The issue is the driver
should somehow differ which USB controller it's bound too and do different
things depending on that (at least that's how it looks now). I'm not sure it's
possible, so probably should be rethought.

> based on a syscon driver.

    I don't know what syscon is...

> In all honesty I don't see that happening on davinci.

    I don't think people use USB 1.1 controllers these days, especially when
there is USB 2.0 in the same SoC. For MUSB, there were recent attempt to get
the DA8xx driver out of the broken state but it was turned down as well, IIRC,
since it didn't offer a PHY driver.

> A somewhat better approach would be to export a set of exported
> functions to access the one register from the platform, e.g.

> u32 da8xx_cfgchip2_get(void);
> void da8xx_cfgchip2_set(u32);

> That interface would still be a bit ugly, but much better than
> what we have today, and easy to implement.

    I'm leaving this idea to Sekhar...

> Arnd

WBR, Sergei

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Arnd Bergmann
On Thursday 20 March 2014 01:36:13 Sergei Shtylyov wrote:
> On 03/19/2014 11:21 PM, Arnd Bergmann wrote:
> > The question is whether there is anyone who would do this properly.
>
>     Nobody cares, it seems. As you can imagine, I stopped to care enough after
> being switched to other projects.

I only care about it because I have the intention to get all 'randconfig'
kernels to build eventually. For stuff that is definitely 'legacy'
and won't get cleaned up properly, I'm fine with a hack.

> > Both the OHCI and MUSB drivers use exactly one register (CFGCHIP2)
>
>     The idea at the time was to just ioremap() this register but I was not
> very eager.

Yes, that would work, too. However, that would cause problems later
if we ever try to make the davinci platform "multiplatform", unless we also
pass the physical address in a platform resource and make this a larger
driver. I think we can reasonably have an exported set of functions
declared in the platform data header file for these drivers, but passing
the physical address through a header file wouldn't be much of an improvement
over passing the virtual address.

> There was no USB PHY driver subsystem yet.
>
> > to control the clock, phy
>
>     Note that it only controls PHY clock (PLL) which could be covered by an
> USB PHY driver.

Good point.

> > and host/gadget mode switch.
>
>     The main mode the USB 2.0 PHY should function now is OTG. The host/gadged
> modes are forced overrides of the ID pin. Unfortunately, the board code has to
> force host mode when host-only driver config is selected (these MUSB's
> host/gadget only modes were removed at one point but got reintroduced again).

I think they are also required for 'randconfig' builds to some degree, because
you can build a kernel without host mode for instance.

> > In the modern world, we'd probably want to have a clock driver and
>
>     Not sure about the clock driver...
>
> > a phy driver for these,
>
>     Yes, that's what the MUSB maintainer wants too. The issue is the driver
> should somehow differ which USB controller it's bound too and do different
> things depending on that (at least that's how it looks now). I'm not sure it's
> possible, so probably should be rethought.

Yes, a phy driver seems the right approach if we can find someone to do it.
Ideally that should let us use generic versions of the ohci and musb drivers
even, if that's the only davinci specific part of these drivers.

> > based on a syscon driver.
>
>     I don't know what syscon is...

The syscon (system controller) framework helps share a set of registers
across multiple drivers. If all accesses to the CFGCHIP2 register are
in a single PHY driver, we wouldn't need it.

> > In all honesty I don't see that happening on davinci.
>
>     I don't think people use USB 1.1 controllers these days, especially when
> there is USB 2.0 in the same SoC. For MUSB, there were recent attempt to get
> the DA8xx driver out of the broken state but it was turned down as well, IIRC,
> since it didn't offer a PHY driver.

For USB 2.0 compliance you actually need to provide something to handle the
low speed modes. A lot of people use a hub to do that nowadays rather than
an OHCI or UHCI, but I don't know if that works with OTG.

        Arnd
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Sekhar Nori
In reply to this post by Arnd Bergmann
Hi Arnd,

On Thursday 20 March 2014 01:51 AM, Arnd Bergmann wrote:

> On Wednesday 19 March 2014 23:53:18 Sergei Shtylyov wrote:
>> On 03/19/2014 10:29 PM, Arnd Bergmann wrote:
>>
>>> The ohci-da8xx driver uses the DA8XX_SYSCFG0_VIRT macro to
>>> access the CFGCHIP2 register for controlling its PHY.
>>
>>> The macro in turn relies on the da8xx_syscfg0_base global
>>> variable. Since the OHCI driver can be a loadable module,
>>> this requires the symbol to be exported from platform code.
>>
>>> Signed-off-by: Arnd Bergmann <[hidden email]>
>>> Cc: Sekhar Nori <[hidden email]>
>>> Cc: Kevin Hilman <[hidden email]>
>>> Cc: [hidden email]
>>> ---
>>>   arch/arm/mach-davinci/devices-da8xx.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>
>>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
>>> index 0486cdf..4da868a 100644
>>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>>> @@ -66,6 +66,7 @@
>>>   #define DA850_DMA_MMCSD1_TX EDMA_CTLR_CHAN(1, 29)
>>>
>>>   void __iomem *da8xx_syscfg0_base;
>>> +EXPORT_SYMBOL_GPL(da8xx_syscfg0_base); /* used by OHCI_HCD */
>>
>>     I have submitted such patch years ago and it was turned down.
>>
>
> The question is whether there is anyone who would do this properly.
>
> Both the OHCI and MUSB drivers use exactly one register (CFGCHIP2)
> to control the clock, phy and host/gadget mode switch.
>
> In the modern world, we'd probably want to have a clock driver and
> a phy driver for these, based on a syscon driver.
>
> In all honesty I don't see that happening on davinci.
>
> A somewhat better approach would be to export a set of exported
> functions to access the one register from the platform, e.g.
>
> u32 da8xx_cfgchip2_get(void);
> void da8xx_cfgchip2_set(u32);
>
> That interface would still be a bit ugly, but much better than
> what we have today, and easy to implement.

There is another thing we can do albeit in the driver (see patch).
Not sure how the USB maintainer will feel about it but I think this
has the advantage of not creating any hacky interfaces. And it
leaves me with the hope that someone will find the time to convert
to phy driver based on syscon at some point.

Thanks,
Sekhar

---8<---
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 3586460..c807d3f 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -1178,7 +1178,8 @@ MODULE_LICENSE ("GPL");
 #define SA1111_DRIVER          ohci_hcd_sa1111_driver
 #endif
 
-#ifdef CONFIG_ARCH_DAVINCI_DA8XX
+/* DA8XX uses platform internal symbols. Cannot be built as module. */
+#if defined(CONFIG_ARCH_DAVINCI_DA8XX) && !defined(CONFIG_USB_OHCI_HCD_MODULE)
 #include "ohci-da8xx.c"
 #define DAVINCI_PLATFORM_DRIVER        ohci_hcd_da8xx_driver
 #endif



_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Sekhar Nori
In reply to this post by Arnd Bergmann
On Thursday 20 March 2014 12:12 PM, Arnd Bergmann wrote:

> On Thursday 20 March 2014 01:36:13 Sergei Shtylyov wrote:
>> On 03/19/2014 11:21 PM, Arnd Bergmann wrote:
>>> The question is whether there is anyone who would do this properly.
>>
>>     Nobody cares, it seems. As you can imagine, I stopped to care enough after
>> being switched to other projects.
>
> I only care about it because I have the intention to get all 'randconfig'
> kernels to build eventually. For stuff that is definitely 'legacy'
> and won't get cleaned up properly, I'm fine with a hack.
>
>>> Both the OHCI and MUSB drivers use exactly one register (CFGCHIP2)
>>
>>     The idea at the time was to just ioremap() this register but I was not
>> very eager.
>
> Yes, that would work, too. However, that would cause problems later
> if we ever try to make the davinci platform "multiplatform", unless we also
> pass the physical address in a platform resource and make this a larger

Some SATA driver work done by Bartlomiej Zolnierkiewicz needed driver
access to syscfg registers too and I just asked him to pass the physical
addresses using platform resource. I think thats the best bet we have in
the absence of a modern interface.

> The syscon (system controller) framework helps share a set of registers
> across multiple drivers. If all accesses to the CFGCHIP2 register are
> in a single PHY driver, we wouldn't need it.

CFGCHIP2 has controls both for USB 1.1 (OHCI) and USB 2.0 (MUSB). Not
sure if there can be a single PHY driver for both.

Thanks,
Sekhar
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Arnd Bergmann
On Thursday 20 March 2014, Sekhar Nori wrote:

> On Thursday 20 March 2014 12:12 PM, Arnd Bergmann wrote:
> > On Thursday 20 March 2014 01:36:13 Sergei Shtylyov wrote:
> >> On 03/19/2014 11:21 PM, Arnd Bergmann wrote:
> >>> The question is whether there is anyone who would do this properly.
> >>
> >>     Nobody cares, it seems. As you can imagine, I stopped to care enough after
> >> being switched to other projects.
> >
> > I only care about it because I have the intention to get all 'randconfig'
> > kernels to build eventually. For stuff that is definitely 'legacy'
> > and won't get cleaned up properly, I'm fine with a hack.
> >
> >>> Both the OHCI and MUSB drivers use exactly one register (CFGCHIP2)
> >>
> >>     The idea at the time was to just ioremap() this register but I was not
> >> very eager.
> >
> > Yes, that would work, too. However, that would cause problems later
> > if we ever try to make the davinci platform "multiplatform", unless we also
> > pass the physical address in a platform resource and make this a larger
>
> Some SATA driver work done by Bartlomiej Zolnierkiewicz needed driver
> access to syscfg registers too and I just asked him to pass the physical
> addresses using platform resource. I think thats the best bet we have in
> the absence of a modern interface.

Ok.

> > The syscon (system controller) framework helps share a set of registers
> > across multiple drivers. If all accesses to the CFGCHIP2 register are
> > in a single PHY driver, we wouldn't need it.
>
> CFGCHIP2 has controls both for USB 1.1 (OHCI) and USB 2.0 (MUSB). Not
> sure if there can be a single PHY driver for both.

The phy infrastructure explicitly supports multiple consumers for one
phy, if I'm reading the code correctly.

        Arnd
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Arnd Bergmann
In reply to this post by Sekhar Nori
On Thursday 20 March 2014, Sekhar Nori wrote:
> There is another thing we can do albeit in the driver (see patch).
> Not sure how the USB maintainer will feel about it but I think this
> has the advantage of not creating any hacky interfaces. And it
> leaves me with the hope that someone will find the time to convert
> to phy driver based on syscon at some point.

Interesting hack.

> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
> index 3586460..c807d3f 100644
> --- a/drivers/usb/host/ohci-hcd.c
> +++ b/drivers/usb/host/ohci-hcd.c
> @@ -1178,7 +1178,8 @@ MODULE_LICENSE ("GPL");
>  #define SA1111_DRIVER          ohci_hcd_sa1111_driver
>  #endif
>  
> -#ifdef CONFIG_ARCH_DAVINCI_DA8XX
> +/* DA8XX uses platform internal symbols. Cannot be built as module. */
> +#if defined(CONFIG_ARCH_DAVINCI_DA8XX) && !defined(CONFIG_USB_OHCI_HCD_MODULE)
>  #include "ohci-da8xx.c"
>  #define DAVINCI_PLATFORM_DRIVER        ohci_hcd_da8xx_driver
>  #endif

I wouldn't want to submit that patch to GregKH ;-)

How about doing the same thing in a somewhat less sneaky way?

Signed-off-by: Arnd Bergmann <[hidden email]>

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 0fe936c..857250a 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -417,6 +417,16 @@ config USB_OHCI_HCD_OMAP3
   Enables support for the on-chip OHCI controller on
   OMAP3 and later chips.
 
+config USB_OHCI_HCD_DAVINCI
+ bool "OHCI support for TI DaVinci DA8xx"
+ depends on ARCH_DAVINCI_DA8XX
+ depends on USB_OHCI_HCD=y
+ default y
+ help
+  Enables support for the DaVinci DA8xx integrated OHCI
+  controller. This driver cannot currently be a loadable
+  module because it lacks a proper PHY abstraction.
+
 config USB_OHCI_ATH79
  bool "USB OHCI support for the Atheros AR71XX/AR7240 SoCs (DEPRECATED)"
  depends on (SOC_AR71XX || SOC_AR724X)
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 3586460..f98d03f 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -1178,7 +1178,7 @@ MODULE_LICENSE ("GPL");
 #define SA1111_DRIVER ohci_hcd_sa1111_driver
 #endif
 
-#ifdef CONFIG_ARCH_DAVINCI_DA8XX
+#ifdef CONFIG_USB_OHCI_HCD_DAVINCI
 #include "ohci-da8xx.c"
 #define DAVINCI_PLATFORM_DRIVER ohci_hcd_da8xx_driver
 #endif
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Sekhar Nori
On Thursday 20 March 2014 05:27 PM, Arnd Bergmann wrote:
> On Thursday 20 March 2014, Sekhar Nori wrote:

>> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
>> index 3586460..c807d3f 100644
>> --- a/drivers/usb/host/ohci-hcd.c
>> +++ b/drivers/usb/host/ohci-hcd.c
>> @@ -1178,7 +1178,8 @@ MODULE_LICENSE ("GPL");
>>  #define SA1111_DRIVER          ohci_hcd_sa1111_driver
>>  #endif
>>  
>> -#ifdef CONFIG_ARCH_DAVINCI_DA8XX
>> +/* DA8XX uses platform internal symbols. Cannot be built as module. */
>> +#if defined(CONFIG_ARCH_DAVINCI_DA8XX) && !defined(CONFIG_USB_OHCI_HCD_MODULE)
>>  #include "ohci-da8xx.c"
>>  #define DAVINCI_PLATFORM_DRIVER        ohci_hcd_da8xx_driver
>>  #endif
>
> I wouldn't want to submit that patch to GregKH ;-)
>
> How about doing the same thing in a somewhat less sneaky way?
>
> Signed-off-by: Arnd Bergmann <[hidden email]>

Much better! Please feel free to add

Acked-by: Sekhar Nori <[hidden email]>

if it helps.

Regards,
Sekhar

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Sergei Shtylyov-3
In reply to this post by Arnd Bergmann
Hello.

On 20-03-2014 10:42, Arnd Bergmann wrote:

>>> Both the OHCI and MUSB drivers use exactly one register (CFGCHIP2)

>>      The idea at the time was to just ioremap() this register but I was not
>> very eager.

    ... also it was suggested to pass the CFGCHIP2 address as a resource. I
certainly wasn't eager to do that since if done for both MUSB and OHCI, it
would cause sort of a resource conflict (platform device resources are
automatically registered in the resource tree, although without marking them
exclusive), even if we don't call request_mem_region() on them (we can't do
that since in this case the resource would be registered as exclusive, and the
second call would fail).

> Yes, that would work, too. However, that would cause problems later
> if we ever try to make the davinci platform "multiplatform", unless we also
> pass the physical address in a platform resource and make this a larger
> driver.

    In my opinion, we don't have to pass CFGCHIP2 as a resource and could just
ioremap() the raw physical address.

> I think we can reasonably have an exported set of functions
> declared in the platform data header file for these drivers, but passing
> the physical address through a header file

    That's how it's passed today (however, thru <mach/da8xx.h>).

> wouldn't be much of an improvement
> over passing the virtual address.

    Not sure I understood about passing virtual address.

>> There was no USB PHY driver subsystem yet.

>>> to control the clock, phy

>>      Note that it only controls PHY clock (PLL) which could be covered by an
>> USB PHY driver.

> Good point.

>>> and host/gadget mode switch.

>>      The main mode the USB 2.0 PHY should function now is OTG. The host/gadged
>> modes are forced overrides of the ID pin. Unfortunately, the board code has to
>> force host mode when host-only driver config is selected (these MUSB's
>> host/gadget only modes were removed at one point but got reintroduced again).

> I think they are also required for 'randconfig' builds to some degree, because
> you can build a kernel without host mode for instance.

    Yes.

>>> In the modern world, we'd probably want to have a clock driver and

>>      Not sure about the clock driver...

>>> a phy driver for these,

>>      Yes, that's what the MUSB maintainer wants too. The issue is the driver
>> should somehow differ which USB controller it's bound too and do different
>> things depending on that (at least that's how it looks now). I'm not sure it's
>> possible, so probably should be rethought.

> Yes, a phy driver seems the right approach if we can find someone to do it.

    Perhaps a person that tried to unbreak the DA8xx MUSB driver could be
interested (and competent) enough to do it...

> Ideally that should let us use generic versions of the ohci and musb drivers
> even, if that's the only davinci specific part of these drivers.

    No, not really. MUSB ceratinly has DaVinci specific register set mapped in
its register space. OHCI has up to 2 clocks to enable since USB 1.1 PHY can be
clocked from USB 2.0 PHY clock.

>>> based on a syscon driver.

>>      I don't know what syscon is...

> The syscon (system controller) framework helps share a set of registers
> across multiple drivers. If all accesses to the CFGCHIP2 register are
> in a single PHY driver, we wouldn't need it.

    Where can I find it in the kernel tree?

>>> In all honesty I don't see that happening on davinci.

>>      I don't think people use USB 1.1 controllers these days, especially when
>> there is USB 2.0 in the same SoC. For MUSB, there were recent attempt to get
>> the DA8xx driver out of the broken state but it was turned down as well, IIRC,
>> since it didn't offer a PHY driver.

> For USB 2.0 compliance you actually need to provide something to handle the
> low speed modes. A lot of people use a hub to do that nowadays rather than
> an OHCI or UHCI, but I don't know if that works with OTG.

    MUSB handles all speeds. I think the reason TI also included USB 1.1
controller lies in the somewhat dubious reputation of both MUSB hardware and
the Linux driver.

> Arnd

WBR, Sergei

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Sergei Shtylyov-3
In reply to this post by Arnd Bergmann
Hello.

On 03/20/2014 02:50 PM, Arnd Bergmann wrote:

>>>>> The question is whether there is anyone who would do this properly.

>>>>      Nobody cares, it seems. As you can imagine, I stopped to care enough after
>>>> being switched to other projects.

>>> I only care about it because I have the intention to get all 'randconfig'
>>> kernels to build eventually. For stuff that is definitely 'legacy'
>>> and won't get cleaned up properly, I'm fine with a hack.

>>>>> Both the OHCI and MUSB drivers use exactly one register (CFGCHIP2)

>>>>      The idea at the time was to just ioremap() this register but I was not
>>>> very eager.

>>> Yes, that would work, too. However, that would cause problems later
>>> if we ever try to make the davinci platform "multiplatform", unless we also
>>> pass the physical address in a platform resource and make this a larger

>> Some SATA driver work done by Bartlomiej Zolnierkiewicz needed driver
>> access to syscfg registers too and I just asked him to pass the physical
>> addresses using platform resource. I think thats the best bet we have in
>> the absence of a modern interface.

> Ok.

    Depends on what SYSCFG register it uses. It wouldn't be good if that
register range includes CFGCHIP2.

>>> The syscon (system controller) framework helps share a set of registers
>>> across multiple drivers. If all accesses to the CFGCHIP2 register are
>>> in a single PHY driver, we wouldn't need it.

>> CFGCHIP2 has controls both for USB 1.1 (OHCI) and USB 2.0 (MUSB). Not
>> sure if there can be a single PHY driver for both.

> The phy infrastructure explicitly supports multiple consumers for one
> phy, if I'm reading the code correctly.

    Yes, it does. The issue is that the PHY code is different in MUSB and OHCI
drivers. I don't think the PHY driver knows what device binds to it.

> Arnd

WBR, Sergei

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Arnd Bergmann
On Thursday 20 March 2014 21:59:29 Sergei Shtylyov wrote:
>
>     Yes, it does. The issue is that the PHY code is different in MUSB and OHCI
> drivers. I don't think the PHY driver knows what device binds to it.

At least in the DT case, it can get that information from DT, when you
set #phy-cells=<1>. I don't know how it would be done for platform_data,
but I assume one could find a way too.

        Arnd
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Sergei Shtylyov-3
Hello.

On 03/20/2014 09:22 PM, Arnd Bergmann wrote:

>>      Yes, it does. The issue is that the PHY code is different in MUSB and OHCI
>> drivers. I don't think the PHY driver knows what device binds to it.

> At least in the DT case, it can get that information from DT, when you
> set #phy-cells=<1>. I don't know how it would be done for platform_data,
> but I assume one could find a way too.

    #phy-cells is only defined for drivers/phy/ bindings IIRC, and I was
thinking a drivers/usb/phy/ driver so far. Probably you have a point and we
should go for the generic PHY framework instead. I'm just not very familiar
with it...

> Arnd

WBR, Sergei

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base

Sergei Shtylyov-3
In reply to this post by Sergei Shtylyov-3
On 03/20/2014 07:20 PM, Sergei Shtylyov wrote:

>> The syscon (system controller) framework helps share a set of registers
>> across multiple drivers. If all accesses to the CFGCHIP2 register are
>> in a single PHY driver, we wouldn't need it.

>     Where can I find it in the kernel tree?

    Found it.

WBR, Sergei

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source