All of lore.kernel.org
 help / color / mirror / Atom feed
From: Evgeny Boger <boger@wirenboard.com>
To: qianfan <qianfanguijin@163.com>
Cc: andre.przywara@arm.com, devicetree@vger.kernel.org,
	jernej.skrabec@gmail.com, krzysztof.kozlowski+dt@linaro.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-sunxi@lists.linux.dev,
	robh+dt@kernel.org, samuel@sholland.org, wens@csie.org
Subject: Re: [PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support
Date: Sat, 21 May 2022 14:10:25 +0300	[thread overview]
Message-ID: <21a659db-f64c-fbea-b3e7-ccc27aec29a0@wirenboard.com> (raw)
In-Reply-To: <0936de49-a349-8fd4-2598-2a8995b01c86@163.com>

Hi,

my point is not that your patch is good enough.

My point is that it would be very difficult to maintain backward 
compatibility with this device tree once proper driver support is 
implemented.


On 5/21/22 07:26, qianfan wrote:
>
>
> 在 2022/5/20 4:54, Evgeny Boger 写道:
>> Hi qianfan,
>>
>> As Allwinner A40i user, let me first thank you for your effort for 
>> making better upstream support for R40!
>>
>> However, I would strongly suggest *not* to add USB support to one 
>> more Allwinner SoC in this particular way.
>> The problem is, this approach consists of a number of carefully 
>> crafted hacks in device tree to make current drivers work on 
>> Allwinner hardware without modification to the drivers.
>>
>> a few examples:
>>
>> 1) please notice how ohci0 and ehci0 nodes do not contain reference 
>> to usb phy. It is done intentionally, otherwise EHCI will reset musb 
>> mode.
>> Of course omitting phy reference here is also completely breaking 
>> power cycling in case of usb error and otherwise messes with a power 
>> management.
>>
>> 2) one must always enable ohci, ehci and usb_otg nodes at the same 
>> time. If one forgets to enable ohci/ehci nodes while enabling usb_otg 
>> node, the system will silently fail to work as USB host.
>>
>> 3) For host-only mode we still have to enable usb_otg node despite no 
>> role switching is needed. That's because phy reference is missing in 
>> ehci/ohci, so the ehci/ohci driver won't enable the PHY.
>> Also I might be wrong, but I think phy won't be routed to ehci/ohci 
>> controllers is this case.
>>
>> 4) musb host controller is initialized and present to hardware though 
>> never actually used
>>
>> To summarize, not only the resulting device tree is not describing 
>> the hardware properly, it is creating device tree configuration which 
>> will be very hard to support in future, once proper driver support is 
>> in place.
> PHY setting is did in MUSB driver, so we need enable MUSB regardless 
> of host mode.
>
> I know your's point, OHCI/EHCI need do more works to init USBPHY, it 
> shoule be able to work
> in dependently, but I don't have the ability to deal with these things 
> right now, I need
> learn more things about OHCI/EHCI, that's a long-term goal.
>
> So now I need to make the whole usb work and do some tests as much as 
> possible,
> hoping to merge this patch into master. Some other optimizations can 
> be made later.
>
> Thanks for yours guide.
>>
>>
>> At Wiren Board kernel tree we tried to untangle this issue [1-6]. 
>> Unfortunately I didn't have time to prepare it for kernel submission 
>> yet, but I think I better submit it as RFC to get a feedback from you 
>> and others.
>>
>>
>> [1] 
>> https://github.com/wirenboard/linux/commit/359abbbd86ddff4d3c61179c882c286de32bb089
>> [2] 
>> https://github.com/wirenboard/linux/commit/6327f9d7972c21b229fb83457fdde643b31553f9
>> [3] 
>> https://github.com/wirenboard/linux/commit/f01f4c66758bde460a4d8c5b54ecee3b585c0232
>> [4] 
>> https://github.com/wirenboard/linux/commit/c27598ad601e5a46f624b73412a531d6f1f63d37
>> [5] 
>> https://github.com/wirenboard/linux/commit/5796d6eebb86b32a3751b2038b63af46f94954b3
>> [6] 
>> https://github.com/wirenboard/linux/commit/0928a675d875f9c2849fd3a9888f718bbb673bda
>>
>>
>


-- 
Kind regards,
Evgeny Boger
CTO @ Wiren Board
+49 3046690053
https://wirenboard.com/


WARNING: multiple messages have this Message-ID (diff)
From: Evgeny Boger <boger@wirenboard.com>
To: qianfan <qianfanguijin@163.com>
Cc: andre.przywara@arm.com, devicetree@vger.kernel.org,
	jernej.skrabec@gmail.com, krzysztof.kozlowski+dt@linaro.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-sunxi@lists.linux.dev,
	robh+dt@kernel.org, samuel@sholland.org, wens@csie.org
Subject: Re: [PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support
Date: Sat, 21 May 2022 14:10:25 +0300	[thread overview]
Message-ID: <21a659db-f64c-fbea-b3e7-ccc27aec29a0@wirenboard.com> (raw)
In-Reply-To: <0936de49-a349-8fd4-2598-2a8995b01c86@163.com>

Hi,

my point is not that your patch is good enough.

My point is that it would be very difficult to maintain backward 
compatibility with this device tree once proper driver support is 
implemented.


On 5/21/22 07:26, qianfan wrote:
>
>
> 在 2022/5/20 4:54, Evgeny Boger 写道:
>> Hi qianfan,
>>
>> As Allwinner A40i user, let me first thank you for your effort for 
>> making better upstream support for R40!
>>
>> However, I would strongly suggest *not* to add USB support to one 
>> more Allwinner SoC in this particular way.
>> The problem is, this approach consists of a number of carefully 
>> crafted hacks in device tree to make current drivers work on 
>> Allwinner hardware without modification to the drivers.
>>
>> a few examples:
>>
>> 1) please notice how ohci0 and ehci0 nodes do not contain reference 
>> to usb phy. It is done intentionally, otherwise EHCI will reset musb 
>> mode.
>> Of course omitting phy reference here is also completely breaking 
>> power cycling in case of usb error and otherwise messes with a power 
>> management.
>>
>> 2) one must always enable ohci, ehci and usb_otg nodes at the same 
>> time. If one forgets to enable ohci/ehci nodes while enabling usb_otg 
>> node, the system will silently fail to work as USB host.
>>
>> 3) For host-only mode we still have to enable usb_otg node despite no 
>> role switching is needed. That's because phy reference is missing in 
>> ehci/ohci, so the ehci/ohci driver won't enable the PHY.
>> Also I might be wrong, but I think phy won't be routed to ehci/ohci 
>> controllers is this case.
>>
>> 4) musb host controller is initialized and present to hardware though 
>> never actually used
>>
>> To summarize, not only the resulting device tree is not describing 
>> the hardware properly, it is creating device tree configuration which 
>> will be very hard to support in future, once proper driver support is 
>> in place.
> PHY setting is did in MUSB driver, so we need enable MUSB regardless 
> of host mode.
>
> I know your's point, OHCI/EHCI need do more works to init USBPHY, it 
> shoule be able to work
> in dependently, but I don't have the ability to deal with these things 
> right now, I need
> learn more things about OHCI/EHCI, that's a long-term goal.
>
> So now I need to make the whole usb work and do some tests as much as 
> possible,
> hoping to merge this patch into master. Some other optimizations can 
> be made later.
>
> Thanks for yours guide.
>>
>>
>> At Wiren Board kernel tree we tried to untangle this issue [1-6]. 
>> Unfortunately I didn't have time to prepare it for kernel submission 
>> yet, but I think I better submit it as RFC to get a feedback from you 
>> and others.
>>
>>
>> [1] 
>> https://github.com/wirenboard/linux/commit/359abbbd86ddff4d3c61179c882c286de32bb089
>> [2] 
>> https://github.com/wirenboard/linux/commit/6327f9d7972c21b229fb83457fdde643b31553f9
>> [3] 
>> https://github.com/wirenboard/linux/commit/f01f4c66758bde460a4d8c5b54ecee3b585c0232
>> [4] 
>> https://github.com/wirenboard/linux/commit/c27598ad601e5a46f624b73412a531d6f1f63d37
>> [5] 
>> https://github.com/wirenboard/linux/commit/5796d6eebb86b32a3751b2038b63af46f94954b3
>> [6] 
>> https://github.com/wirenboard/linux/commit/0928a675d875f9c2849fd3a9888f718bbb673bda
>>
>>
>


-- 
Kind regards,
Evgeny Boger
CTO @ Wiren Board
+49 3046690053
https://wirenboard.com/


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-05-21 11:10 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-18 10:17 [PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support qianfanguijin
2022-05-18 10:17 ` qianfanguijin
2022-05-18 10:17 ` [PATCH v4 1/2] ARM: dts: sun8i-r40: Add USB0_OTG/HOST support qianfanguijin
2022-05-18 10:17   ` qianfanguijin
2022-07-05  1:55   ` Samuel Holland
2022-07-05  1:55     ` Samuel Holland
2022-05-18 10:17 ` [PATCH v4 2/2] ARM: dts: bananapi-m2-ultra: Enable USB0_OTG and HOST support qianfanguijin
2022-05-18 10:17   ` qianfanguijin
2022-07-05  2:38   ` Samuel Holland
2022-07-05  2:38     ` Samuel Holland
2022-05-19 10:04 ` [PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support qianfan
2022-05-19 10:04   ` qianfan
2022-05-19 20:54   ` Evgeny Boger
2022-05-19 20:54     ` Evgeny Boger
2022-05-21  4:26     ` qianfan
2022-05-21  4:26       ` qianfan
2022-05-21 11:10       ` Evgeny Boger [this message]
2022-05-21 11:10         ` Evgeny Boger
2022-07-05  4:05         ` Samuel Holland
2022-07-05  4:05           ` Samuel Holland
2022-05-23 13:10     ` Icenowy Zheng
2022-05-23 13:10       ` Icenowy Zheng
2022-05-23 13:11 ` Icenowy Zheng
2022-05-23 13:11   ` Icenowy Zheng
2022-05-24  9:15   ` qianfan
2022-05-24  9:15     ` qianfan
2022-07-05  4:15   ` Samuel Holland
2022-07-05  4:15     ` Samuel Holland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=21a659db-f64c-fbea-b3e7-ccc27aec29a0@wirenboard.com \
    --to=boger@wirenboard.com \
    --cc=andre.przywara@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jernej.skrabec@gmail.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=qianfanguijin@163.com \
    --cc=robh+dt@kernel.org \
    --cc=samuel@sholland.org \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.