Re: [PATCH] ARM: AM35xx: Add M-USB clk device ID

[Tero Kristo]

On 10/12/2015 06:22 PM, Rolf Peukert wrote:

The glue code in drivers/usb/musb/am35x.c calls clk_get() to get its
interface and function clocks for the M-USB controller. These calls fail
in the current kernel. This patch adds clock definitions containing the
device ID to the list in clk-3xxx.c, so the calls to clk_get() in
am35x.c can succeed.

Signed-off-by:  Rolf Peukert 

---
  drivers/clk/ti/clk-3xxx.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/clk/ti/clk-3xxx.c b/drivers/clk/ti/clk-3xxx.c
index 8831e1a..b635deb 100644
--- a/drivers/clk/ti/clk-3xxx.c
+++ b/drivers/clk/ti/clk-3xxx.c
@@ -507,7 +507,9 @@ static struct ti_dt_clk am35xx_clks[] = {
DT_CLK("davinci_mdio.0", NULL, "emac_fck"),
DT_CLK("vpfe-capture", "master", "vpfe_ick"),
DT_CLK("vpfe-capture", "slave", "vpfe_fck"),
+   DT_CLK("5c04.am35x_otg_hs", "ick", "hsotgusb_ick_am35xx"),
DT_CLK(NULL, "hsotgusb_ick", "hsotgusb_ick_am35xx"),
+   DT_CLK("5c04.am35x_otg_hs", "fck", "hsotgusb_fck_am35xx"),
DT_CLK(NULL, "hsotgusb_fck", "hsotgusb_fck_am35xx"),
DT_CLK(NULL, "hecc_ck", "hecc_ck"),
DT_CLK(NULL, "uart4_ick", "uart4_ick_am35xx"),



Adding clock aliases should be avoided, isn't there any other way to fix 
this issue? Like adding clocks = <> references under the DT node?


-Tero

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: AM35xx: Add M-USB clk device ID

[Rolf Peukert]

On 13.10.2015 10:15, Tero Kristo wrote:
> On 10/12/2015 06:22 PM, Rolf Peukert wrote:
>> The glue code in drivers/usb/musb/am35x.c calls clk_get() to get its
>> interface and function clocks for the M-USB controller. These calls fail
>> in the current kernel. This patch adds clock definitions containing the
>> device ID to the list in clk-3xxx.c, so the calls to clk_get() in
>> am35x.c can succeed.
>>
>> Signed-off-by:  Rolf Peukert 
>>
>> ---
>>   drivers/clk/ti/clk-3xxx.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/clk/ti/clk-3xxx.c b/drivers/clk/ti/clk-3xxx.c
>> index 8831e1a..b635deb 100644
>> --- a/drivers/clk/ti/clk-3xxx.c
>> +++ b/drivers/clk/ti/clk-3xxx.c
>> @@ -507,7 +507,9 @@ static struct ti_dt_clk am35xx_clks[] = {
>>   DT_CLK("davinci_mdio.0", NULL, "emac_fck"),
>>   DT_CLK("vpfe-capture", "master", "vpfe_ick"),
>>   DT_CLK("vpfe-capture", "slave", "vpfe_fck"),
>> +DT_CLK("5c04.am35x_otg_hs", "ick", "hsotgusb_ick_am35xx"),
>>   DT_CLK(NULL, "hsotgusb_ick", "hsotgusb_ick_am35xx"),
>> +DT_CLK("5c04.am35x_otg_hs", "fck", "hsotgusb_fck_am35xx"),
>>   DT_CLK(NULL, "hsotgusb_fck", "hsotgusb_fck_am35xx"),
>>   DT_CLK(NULL, "hecc_ck", "hecc_ck"),
>>   DT_CLK(NULL, "uart4_ick", "uart4_ick_am35xx"),
>>
> 
> Adding clock aliases should be avoided, isn't there any other way to fix
> this issue? Like adding clocks = <> references under the DT node?
> 
> -Tero
> 

Yes, I just tried adding the lines

clocks = <_ick_am35xx>, <_fck_am35xx>;
clock-names = "ick", "fck";

to am3517.dtsi and this works too. But wouldn't this mean the driver
will not work anymore in kernels without DT support?
(my first idea was to change the clk_get calls in am35x.c, but Felipe
Balbi objected)

Best regards,
Rolf

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: AM35xx: Add M-USB clk device ID

[Felipe Balbi]

Hi,

Rolf Peukert  writes:
> On 13.10.2015 10:15, Tero Kristo wrote:
>> On 10/12/2015 06:22 PM, Rolf Peukert wrote:
>>> The glue code in drivers/usb/musb/am35x.c calls clk_get() to get its
>>> interface and function clocks for the M-USB controller. These calls fail
>>> in the current kernel. This patch adds clock definitions containing the
>>> device ID to the list in clk-3xxx.c, so the calls to clk_get() in
>>> am35x.c can succeed.
>>>
>>> Signed-off-by:  Rolf Peukert 
>>>
>>> ---
>>>   drivers/clk/ti/clk-3xxx.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/clk/ti/clk-3xxx.c b/drivers/clk/ti/clk-3xxx.c
>>> index 8831e1a..b635deb 100644
>>> --- a/drivers/clk/ti/clk-3xxx.c
>>> +++ b/drivers/clk/ti/clk-3xxx.c
>>> @@ -507,7 +507,9 @@ static struct ti_dt_clk am35xx_clks[] = {
>>>   DT_CLK("davinci_mdio.0", NULL, "emac_fck"),
>>>   DT_CLK("vpfe-capture", "master", "vpfe_ick"),
>>>   DT_CLK("vpfe-capture", "slave", "vpfe_fck"),
>>> +DT_CLK("5c04.am35x_otg_hs", "ick", "hsotgusb_ick_am35xx"),
>>>   DT_CLK(NULL, "hsotgusb_ick", "hsotgusb_ick_am35xx"),
>>> +DT_CLK("5c04.am35x_otg_hs", "fck", "hsotgusb_fck_am35xx"),
>>>   DT_CLK(NULL, "hsotgusb_fck", "hsotgusb_fck_am35xx"),
>>>   DT_CLK(NULL, "hecc_ck", "hecc_ck"),
>>>   DT_CLK(NULL, "uart4_ick", "uart4_ick_am35xx"),
>>>
>> 
>> Adding clock aliases should be avoided, isn't there any other way to fix
>> this issue? Like adding clocks = <> references under the DT node?
>> 
>> -Tero
>> 
>
> Yes, I just tried adding the lines
>
>   clocks = <_ick_am35xx>, <_fck_am35xx>;
>   clock-names = "ick", "fck";
>
> to am3517.dtsi and this works too. But wouldn't this mean the driver
> will not work anymore in kernels without DT support?

I have this doubt myself. This will break on non-DT boots and, while
we're trying to move to DT-only, IMO meanwhile we should allow for fixes
to DT and non-DT world. Once the conversion is done, fine.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] ARM: AM35xx: Add M-USB clk device ID

[Sebastian Reichel]

Hi,

On Tue, Oct 13, 2015 at 05:57:59PM -0500, Felipe Balbi wrote:
> Sebastian Reichel  writes:
> > On Tue, Oct 13, 2015 at 10:50:45AM -0500, Felipe Balbi wrote:
> >> Rolf Peukert  writes:
> >> > On 13.10.2015 10:15, Tero Kristo wrote:
> >> >> On 10/12/2015 06:22 PM, Rolf Peukert wrote:
> >> >>> The glue code in drivers/usb/musb/am35x.c calls clk_get() to get its
> >> >>> interface and function clocks for the M-USB controller. These calls 
> >> >>> fail
> >> >>> in the current kernel. This patch adds clock definitions containing the
> >> >>> device ID to the list in clk-3xxx.c, so the calls to clk_get() in
> >> >>> am35x.c can succeed.
> >> >>>
> >> >>> Signed-off-by:  Rolf Peukert 
> >> >>>
> >> >>> ---
> >> >>>   drivers/clk/ti/clk-3xxx.c | 2 ++
> >> >>>   1 file changed, 2 insertions(+)
> >> >>>
> >> >>> diff --git a/drivers/clk/ti/clk-3xxx.c b/drivers/clk/ti/clk-3xxx.c
> >> >>> index 8831e1a..b635deb 100644
> >> >>> --- a/drivers/clk/ti/clk-3xxx.c
> >> >>> +++ b/drivers/clk/ti/clk-3xxx.c
> >> >>> @@ -507,7 +507,9 @@ static struct ti_dt_clk am35xx_clks[] = {
> >> >>>   DT_CLK("davinci_mdio.0", NULL, "emac_fck"),
> >> >>>   DT_CLK("vpfe-capture", "master", "vpfe_ick"),
> >> >>>   DT_CLK("vpfe-capture", "slave", "vpfe_fck"),
> >> >>> +DT_CLK("5c04.am35x_otg_hs", "ick", "hsotgusb_ick_am35xx"),
> >> >>>   DT_CLK(NULL, "hsotgusb_ick", "hsotgusb_ick_am35xx"),
> >> >>> +DT_CLK("5c04.am35x_otg_hs", "fck", "hsotgusb_fck_am35xx"),
> >> >>>   DT_CLK(NULL, "hsotgusb_fck", "hsotgusb_fck_am35xx"),
> >> >>>   DT_CLK(NULL, "hecc_ck", "hecc_ck"),
> >> >>>   DT_CLK(NULL, "uart4_ick", "uart4_ick_am35xx"),
> >> >>>
> >> >> 
> >> >> Adding clock aliases should be avoided, isn't there any other way to fix
> >> >> this issue? Like adding clocks = <> references under the DT node?
> >> >> 
> >> >> -Tero
> >> >> 
> >> >
> >> > Yes, I just tried adding the lines
> >> >
> >> >  clocks = <_ick_am35xx>, <_fck_am35xx>;
> >> >  clock-names = "ick", "fck";
> >> >
> >> > to am3517.dtsi and this works too. But wouldn't this mean the driver
> >> > will not work anymore in kernels without DT support?
> >> 
> >> I have this doubt myself. This will break on non-DT boots and, 
> >> while we're trying to move to DT-only, IMO meanwhile we should
> >> allow for fixes to DT and non-DT world. Once the conversion is
> >> done, fine.
> >
> > Isn't am35xx already DT-only? The remaining omap2+ boards are both
> > omap3430 based:
> 
> yeah, and this system is omap3 based, not am335x based ;-)

Sure, am35xx depends on omap3 and omap3 still supports platform
based booting because of the mentioned boards, but that does not
mean, that am35xx also needs to support legacy booting. From what
I can see, the last am35xx board has been removed in v4.0 [0]
together with the am35xx platform headers.

So while omap3 can still be booted the legacy way, am35xx cannot
without modifying the kernel. Other omap3 based SoCs will initialize
musb's omap2430 gluecode, so they are not affected.

[0] 
https://git.kernel.org/cgit/linux/kernel/git/tmlind/linux-omap.git/tag/?id=omap-for-v3.20/drop-legacy-3517-v2

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH] ARM: AM35xx: Add M-USB clk device ID

[Felipe Balbi]

hi,

Sebastian Reichel  writes:
> Hi,
>
> On Tue, Oct 13, 2015 at 10:50:45AM -0500, Felipe Balbi wrote:
>> Rolf Peukert  writes:
>> > On 13.10.2015 10:15, Tero Kristo wrote:
>> >> On 10/12/2015 06:22 PM, Rolf Peukert wrote:
>> >>> The glue code in drivers/usb/musb/am35x.c calls clk_get() to get its
>> >>> interface and function clocks for the M-USB controller. These calls fail
>> >>> in the current kernel. This patch adds clock definitions containing the
>> >>> device ID to the list in clk-3xxx.c, so the calls to clk_get() in
>> >>> am35x.c can succeed.
>> >>>
>> >>> Signed-off-by:  Rolf Peukert 
>> >>>
>> >>> ---
>> >>>   drivers/clk/ti/clk-3xxx.c | 2 ++
>> >>>   1 file changed, 2 insertions(+)
>> >>>
>> >>> diff --git a/drivers/clk/ti/clk-3xxx.c b/drivers/clk/ti/clk-3xxx.c
>> >>> index 8831e1a..b635deb 100644
>> >>> --- a/drivers/clk/ti/clk-3xxx.c
>> >>> +++ b/drivers/clk/ti/clk-3xxx.c
>> >>> @@ -507,7 +507,9 @@ static struct ti_dt_clk am35xx_clks[] = {
>> >>>   DT_CLK("davinci_mdio.0", NULL, "emac_fck"),
>> >>>   DT_CLK("vpfe-capture", "master", "vpfe_ick"),
>> >>>   DT_CLK("vpfe-capture", "slave", "vpfe_fck"),
>> >>> +DT_CLK("5c04.am35x_otg_hs", "ick", "hsotgusb_ick_am35xx"),
>> >>>   DT_CLK(NULL, "hsotgusb_ick", "hsotgusb_ick_am35xx"),
>> >>> +DT_CLK("5c04.am35x_otg_hs", "fck", "hsotgusb_fck_am35xx"),
>> >>>   DT_CLK(NULL, "hsotgusb_fck", "hsotgusb_fck_am35xx"),
>> >>>   DT_CLK(NULL, "hecc_ck", "hecc_ck"),
>> >>>   DT_CLK(NULL, "uart4_ick", "uart4_ick_am35xx"),
>> >>>
>> >> 
>> >> Adding clock aliases should be avoided, isn't there any other way to fix
>> >> this issue? Like adding clocks = <> references under the DT node?
>> >> 
>> >> -Tero
>> >> 
>> >
>> > Yes, I just tried adding the lines
>> >
>> >clocks = <_ick_am35xx>, <_fck_am35xx>;
>> >clock-names = "ick", "fck";
>> >
>> > to am3517.dtsi and this works too. But wouldn't this mean the driver
>> > will not work anymore in kernels without DT support?
>> 
>> I have this doubt myself. This will break on non-DT boots and, 
>> while we're trying to move to DT-only, IMO meanwhile we should
>> allow for fixes to DT and non-DT world. Once the conversion is
>> done, fine.
>
> Isn't am35xx already DT-only? The remaining omap2+ boards are both
> omap3430 based:

yeah, and this system is omap3 based, not am335x based ;-)

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] ARM: AM35xx: Add M-USB clk device ID

[Sebastian Reichel]

Hi,

On Tue, Oct 13, 2015 at 10:50:45AM -0500, Felipe Balbi wrote:
> Rolf Peukert  writes:
> > On 13.10.2015 10:15, Tero Kristo wrote:
> >> On 10/12/2015 06:22 PM, Rolf Peukert wrote:
> >>> The glue code in drivers/usb/musb/am35x.c calls clk_get() to get its
> >>> interface and function clocks for the M-USB controller. These calls fail
> >>> in the current kernel. This patch adds clock definitions containing the
> >>> device ID to the list in clk-3xxx.c, so the calls to clk_get() in
> >>> am35x.c can succeed.
> >>>
> >>> Signed-off-by:  Rolf Peukert 
> >>>
> >>> ---
> >>>   drivers/clk/ti/clk-3xxx.c | 2 ++
> >>>   1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/drivers/clk/ti/clk-3xxx.c b/drivers/clk/ti/clk-3xxx.c
> >>> index 8831e1a..b635deb 100644
> >>> --- a/drivers/clk/ti/clk-3xxx.c
> >>> +++ b/drivers/clk/ti/clk-3xxx.c
> >>> @@ -507,7 +507,9 @@ static struct ti_dt_clk am35xx_clks[] = {
> >>>   DT_CLK("davinci_mdio.0", NULL, "emac_fck"),
> >>>   DT_CLK("vpfe-capture", "master", "vpfe_ick"),
> >>>   DT_CLK("vpfe-capture", "slave", "vpfe_fck"),
> >>> +DT_CLK("5c04.am35x_otg_hs", "ick", "hsotgusb_ick_am35xx"),
> >>>   DT_CLK(NULL, "hsotgusb_ick", "hsotgusb_ick_am35xx"),
> >>> +DT_CLK("5c04.am35x_otg_hs", "fck", "hsotgusb_fck_am35xx"),
> >>>   DT_CLK(NULL, "hsotgusb_fck", "hsotgusb_fck_am35xx"),
> >>>   DT_CLK(NULL, "hecc_ck", "hecc_ck"),
> >>>   DT_CLK(NULL, "uart4_ick", "uart4_ick_am35xx"),
> >>>
> >> 
> >> Adding clock aliases should be avoided, isn't there any other way to fix
> >> this issue? Like adding clocks = <> references under the DT node?
> >> 
> >> -Tero
> >> 
> >
> > Yes, I just tried adding the lines
> >
> > clocks = <_ick_am35xx>, <_fck_am35xx>;
> > clock-names = "ick", "fck";
> >
> > to am3517.dtsi and this works too. But wouldn't this mean the driver
> > will not work anymore in kernels without DT support?
> 
> I have this doubt myself. This will break on non-DT boots and, 
> while we're trying to move to DT-only, IMO meanwhile we should
> allow for fixes to DT and non-DT world. Once the conversion is
> done, fine.

Isn't am35xx already DT-only? The remaining omap2+ boards are both
omap3430 based:

$ grep "^MACHINE_START(" arch/arm/mach-omap2/*
arch/arm/mach-omap2/board-ldp.c:MACHINE_START(OMAP_LDP, "OMAP LDP board")
arch/arm/mach-omap2/board-rx51.c:MACHINE_START(NOKIA_RX51, "Nokia RX-51 board")

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH] ARM: AM35xx: Add M-USB clk device ID

[Felipe Balbi]

Hi,

Sebastian Reichel  writes:
> On Tue, Oct 13, 2015 at 05:57:59PM -0500, Felipe Balbi wrote:
>> Sebastian Reichel  writes:
>> > On Tue, Oct 13, 2015 at 10:50:45AM -0500, Felipe Balbi wrote:
>> >> Rolf Peukert  writes:
>> >> > On 13.10.2015 10:15, Tero Kristo wrote:
>> >> >> On 10/12/2015 06:22 PM, Rolf Peukert wrote:
>> >> >>> The glue code in drivers/usb/musb/am35x.c calls clk_get() to get its
>> >> >>> interface and function clocks for the M-USB controller. These calls 
>> >> >>> fail
>> >> >>> in the current kernel. This patch adds clock definitions containing 
>> >> >>> the
>> >> >>> device ID to the list in clk-3xxx.c, so the calls to clk_get() in
>> >> >>> am35x.c can succeed.
>> >> >>>
>> >> >>> Signed-off-by:  Rolf Peukert 
>> >> >>>
>> >> >>> ---
>> >> >>>   drivers/clk/ti/clk-3xxx.c | 2 ++
>> >> >>>   1 file changed, 2 insertions(+)
>> >> >>>
>> >> >>> diff --git a/drivers/clk/ti/clk-3xxx.c b/drivers/clk/ti/clk-3xxx.c
>> >> >>> index 8831e1a..b635deb 100644
>> >> >>> --- a/drivers/clk/ti/clk-3xxx.c
>> >> >>> +++ b/drivers/clk/ti/clk-3xxx.c
>> >> >>> @@ -507,7 +507,9 @@ static struct ti_dt_clk am35xx_clks[] = {
>> >> >>>   DT_CLK("davinci_mdio.0", NULL, "emac_fck"),
>> >> >>>   DT_CLK("vpfe-capture", "master", "vpfe_ick"),
>> >> >>>   DT_CLK("vpfe-capture", "slave", "vpfe_fck"),
>> >> >>> +DT_CLK("5c04.am35x_otg_hs", "ick", "hsotgusb_ick_am35xx"),
>> >> >>>   DT_CLK(NULL, "hsotgusb_ick", "hsotgusb_ick_am35xx"),
>> >> >>> +DT_CLK("5c04.am35x_otg_hs", "fck", "hsotgusb_fck_am35xx"),
>> >> >>>   DT_CLK(NULL, "hsotgusb_fck", "hsotgusb_fck_am35xx"),
>> >> >>>   DT_CLK(NULL, "hecc_ck", "hecc_ck"),
>> >> >>>   DT_CLK(NULL, "uart4_ick", "uart4_ick_am35xx"),
>> >> >>>
>> >> >> 
>> >> >> Adding clock aliases should be avoided, isn't there any other way to 
>> >> >> fix
>> >> >> this issue? Like adding clocks = <> references under the DT node?
>> >> >> 
>> >> >> -Tero
>> >> >> 
>> >> >
>> >> > Yes, I just tried adding the lines
>> >> >
>> >> > clocks = <_ick_am35xx>, <_fck_am35xx>;
>> >> > clock-names = "ick", "fck";
>> >> >
>> >> > to am3517.dtsi and this works too. But wouldn't this mean the driver
>> >> > will not work anymore in kernels without DT support?
>> >> 
>> >> I have this doubt myself. This will break on non-DT boots and, 
>> >> while we're trying to move to DT-only, IMO meanwhile we should
>> >> allow for fixes to DT and non-DT world. Once the conversion is
>> >> done, fine.
>> >
>> > Isn't am35xx already DT-only? The remaining omap2+ boards are both
>> > omap3430 based:
>> 
>> yeah, and this system is omap3 based, not am335x based ;-)
>
> Sure, am35xx depends on omap3 and omap3 still supports platform
> based booting because of the mentioned boards, but that does not
> mean, that am35xx also needs to support legacy booting. From what
> I can see, the last am35xx board has been removed in v4.0 [0]
> together with the am35xx platform headers.
>
> So while omap3 can still be booted the legacy way, am35xx cannot
> without modifying the kernel. Other omap3 based SoCs will initialize
> musb's omap2430 gluecode, so they are not affected.
>
> [0] 
> https://git.kernel.org/cgit/linux/kernel/git/tmlind/linux-omap.git/tag/?id=omap-for-v3.20/drop-legacy-3517-v2

fair enough, that settles it. DT-only it is.

-- 
balbi


signature.asc
Description: PGP signature

[PATCH] ARM: AM35xx: Add M-USB clk device ID

[Rolf Peukert]

The glue code in drivers/usb/musb/am35x.c calls clk_get() to get its
interface and function clocks for the M-USB controller. These calls fail
in the current kernel. This patch adds clock definitions containing the
device ID to the list in clk-3xxx.c, so the calls to clk_get() in
am35x.c can succeed.

Signed-off-by:  Rolf Peukert 

---
 drivers/clk/ti/clk-3xxx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/ti/clk-3xxx.c b/drivers/clk/ti/clk-3xxx.c
index 8831e1a..b635deb 100644
--- a/drivers/clk/ti/clk-3xxx.c
+++ b/drivers/clk/ti/clk-3xxx.c
@@ -507,7 +507,9 @@ static struct ti_dt_clk am35xx_clks[] = {
DT_CLK("davinci_mdio.0", NULL, "emac_fck"),
DT_CLK("vpfe-capture", "master", "vpfe_ick"),
DT_CLK("vpfe-capture", "slave", "vpfe_fck"),
+   DT_CLK("5c04.am35x_otg_hs", "ick", "hsotgusb_ick_am35xx"),
DT_CLK(NULL, "hsotgusb_ick", "hsotgusb_ick_am35xx"),
+   DT_CLK("5c04.am35x_otg_hs", "fck", "hsotgusb_fck_am35xx"),
DT_CLK(NULL, "hsotgusb_fck", "hsotgusb_fck_am35xx"),
DT_CLK(NULL, "hecc_ck", "hecc_ck"),
DT_CLK(NULL, "uart4_ick", "uart4_ick_am35xx"),
-- 
2.3.6

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html