Re: [RFC PATCH 01/17] sunxi: remove CONFIG_SATAPWR

Samuel Holland
2022-12-14T23:41:02Z

Andre,

On 12/14/22 08:25, Andre Przywara wrote:
> On Wed, 14 Dec 2022 02:37:12 -0600 Samuel Holland <samuel@sholland.org> wrote:
>> On 12/5/22 18:45, Andre Przywara wrote:
>>> diff --git a/configs/Sinovoip_BPI_M3_defconfig b/configs/Sinovoip_BPI_M3_defconfig
>>> index ab70eff68eb..bcc8b1fba98 100644
>>> --- a/configs/Sinovoip_BPI_M3_defconfig
>>> +++ b/configs/Sinovoip_BPI_M3_defconfig
>>> @@ -13,7 +13,6 @@ CONFIG_USB0_VBUS_DET="AXP0-VBUS-DETECT"
>>> CONFIG_USB0_ID_DET="PH11"
>>> CONFIG_USB1_VBUS_PIN="PD24"
>>> CONFIG_AXP_GPIO=y
>>> -CONFIG_SATAPWR="PD25"
>>> # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
>>> CONFIG_SYS_MONITOR_LEN=786432
>>> CONFIG_CONSOLE_MUX=y
>>> diff --git a/configs/orangepi_plus_defconfig b/configs/orangepi_plus_defconfig
>>> index 5c7f0731d90..f4ce4851d7c 100644
>>> --- a/configs/orangepi_plus_defconfig
>>> +++ b/configs/orangepi_plus_defconfig
>>> @@ -7,7 +7,6 @@ CONFIG_DRAM_CLK=672
>>> CONFIG_MACPWR="PD6"
>>> CONFIG_MMC_SUNXI_SLOT_EXTRA=2
>>> CONFIG_USB1_VBUS_PIN="PG13"
>>> -CONFIG_SATAPWR="PG11"
>>
>> BananaPi M3 and OrangePi Plus have USB-SATA adapters, not onboard AHCI,
>> so they would lose the ability to use SATA with this change.
>
> Many thanks for having a thorough look, I much appreciate that.
> Of course you are right. I actually found this myself, and thought I
> mentioned it somewhere, but apparently this got lost in between cover
> letter versions and commit messages on different branches. Apologies for
> that.
> So yeah, my research figured that this isn't described properly in the DT,
> so SATA disks just work in Linux because U-Boot flipped the bit here.
> I heard about USB child DT nodes, which IIUC are possible to describe
> errata and such, but I don't think it's a good solution here. Would
> probably need driver changes, so wouldn't be backwards compatible.
>
>> OrangePi Plus has the SATA controller regulator as usb3_vbus-supply in
>> its devicetree. So we could replace this with CONFIG_USB3_VBUS_PIN for
>> now, and it will continue to work once we switch the PHY driver to use
>> the regulator uclass.
>>
>> But the BananaPi M3 has its USB-SATA downstream from an external hub.
>> CONFIG_USB1_VBUS_PIN is used for the regulator powering the hub, so I do
>> not see an obvious solution here.
>
> Yeah, I didn't find a neat automatic solution for that either.
> Can we add a regulator-fixed, and make this regulator-boot-on? Without
> actually referencing this regulator anywhere? I need to check this
> actually works in U-Boot, but what do you say with your Linux/DT maintainer
> hat on?

I can think of a few solutions, though none is perfect:

1) As you suggest, add a new fixed regulator with regulator-boot-on. It
also needs regulator-always-on to keep the OS from disabling it later,
since it would have no consumer. We would also need to call
regulators_enable_boot_on() from our board code to enable in in U-Boot.

2) Add a new fixed regulator, and point usb1_vbus-supply to it. Then
have the new regulator reference the hub regulator as its vin-supply.
This would require updating the U-Boot fixed regulator driver to control
its vin-supply. And it is also a misleading use of usb1_vbus-supply.

3) Add a gpio-hog to the devicetree, to unconditionally enable the
regulator. This would be the closest equivalent to what we are doing
now, and the quickest path to getting the legacy board code removed. We
could do this entirely within U-Boot, so I lean toward this solution.

Regards,
Samuel