Message-ID: <8841923c-cbb6-4cce-97f4-a851783b6102@freeshell.de>
Date: Sun, 27 Jul 2025 20:52:50 -0700
From: E Shattow <e@...eshell.de>
To: Conor Dooley <conor@...nel.org>
Cc: Emil Renner Berthing <kernel@...il.dk>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 Alexandre Ghiti <alex@...ti.fr>, William Qiu <william.qiu@...rfivetech.com>,
 linux-riscv@...ts.infradead.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Hal Feng <hal.feng@...rfivetech.com>,
 Minda Chen <minda.chen@...rfivetech.com>
Subject: Re: [PATCH] riscv: dts: starfive: jh7110-common: drop no-sdio
 property from mmc1

Adding CC: Hal Feng, Minda Chen

On 7/25/25 11:10, Conor Dooley wrote:
> On Thu, Jul 24, 2025 at 10:13:47PM -0700, E Shattow wrote:
>> On 7/24/25 09:51, Conor Dooley wrote:
>>> On Thu, Jul 24, 2025 at 12:55:53AM -0700, E Shattow wrote:
>>>> Drop no-sdio property avoids a delete-property on variant board dts
>>>> having an SDIO wireless module connected to mmc1.
>>>
>>> I'm struggling to understand why this change is correct.
>>>
>>> If there are specific boards that have wireless modules connected
>>> instead of using sdcards, how come the no-sdio property isn't moved to the
>>> the boards that do have sdcard slots?
>>
>> Why is 'no-sdio' property there to begin with...
>>
>>> The property was added for the visionfive 2, and only on mmc1, so should
>>> it be retained for boards that match the visionfive 2 in terms of how
>>> they use mmc?
>>
>> Ref.:
>> https://lore.kernel.org/lkml/20221207131731.1291517-4-william.qiu@starfivetech.com/
>>
>> My theory is VisionFive2 board hardware can support connecting up some
>> SDIO module there with ready-made available adapters, it may be possible
>> (if unusual) that would work? SDIO is 4-wide and some voltage
>> requirements, and a couple of GPIO, so I'm aware that's a stretch of a
>> statement but it could be done without soldering. I wouldn't expect it,
>> but why restrict this everywhere inheriting from jh7110-common.dtsi with
>> 'no-sdio' and then (needs testing!) if it doesn't matter one way or the
> 
> In case it was not clear, I am not questioning removing it from the
> common file, just that you're removing it entirely.
> 
>> other for VisionFive2 just drop it I think as being inaccurate/unnecessary?
>>
>> JH7110 CPU supports two interfaces of SDIO3.0/eMMC so it's not clear to
>> me if there's some reason for 'no-sdio' property to be there. Does
>> allowing SDIO (?) break eMMC and SD Card devices, is it destructive?
>>
>> Not knowing what 'no-sdio' does technically I dropped the property and
>> tested with the hardware I do have. The 'no-sdio' property
>> present/absent does not appear to do anything user-impactful on Pine64
>> Star64 that has SD Card slot on mmc1, and as would be expected on Milk-V
>> Mars CM Lite WiFi when there's an SDIO module at mmc1 it then fails to
>> initialize if 'no-sdio' property is present.
> 
> The original addition looks very intentional - however I didn't look at
> the commit message itself last night, just the diff. I wonder if "no-sdio"
> and "no-mmc" were just added because William intended to restrict SD
> cards since sdio1 is the SD card slot, rather than because the use of
> sdio or mmc commands during init would cause problems. The wording of "Set
> sdioo node to emmc and set sdio1 node to sd" is what makes me think
> that.
> 
>>> Could you add an explanation for why removing this entirely is the right
>>> thing to do, rather than only removing it for these variant boards?
>>
>> Yes, I can rephrase a bit like "relax no-sdio restriction on mmc1 for
>> jh7110 boards", or else reconsider the approach. I was going to approach
>> with `/delete-property/ no-sdio;` later elsewhere but after testing on
>> Pine64 Star64 with similar configuration to VisionFive2 mmc interfaces,
>> and knowing that Milk-V Mars CM Lite WiFi detects AP6256 SDIO peripheral
>> at mmc1 when this property is dropped (and with a few additional
>> things)... I prefer to reduce the problems that would need to be avoided.
> 
> I think using /delete-property/ would be unwise, properties shouldn't be
> in the common dtsi if they are not, in fact, common.
> 

Ack, you wanted these moved out into each board dts? That would be okay
with me. I suspected that `no-sdio` `no-mmc` don't belong so I'm still
in favor of dropping them overall. I think nothing will break but I want
more data from users.

>> I have done all the testing I can do with hardware I have. As-is it's
>> just like I wrote, we'll have to solicit some testing feedback on that
>> and wait to learn what this does for the other boards.
> 
> I'd kinda be inclined to apply this diff, with a better commit message,
> shortly after -rc1, unless someone comes forward with a justification
> for it being there on the vf2. I figure it's only in the common dtsi
> because it was not problematic until now cos noone tried to use the sdio
> aspect.
> 

I will revise the commit message v2 and send soon, just for dropping the
`no-sdio`...  and actively seek testing reports from users of all the
JH7110 boards to drop `no-sdio` and then also `no-mmc` properties for a
follow-up.

>> Aside, anyone want to chime in what is the utility of 'no-sdio'
>> property, how do you know from a schematic if it is appropriate, can it
>> be simply dropped as I suggest for JH7110 boards?
> 
> My (limited) understanding, mostly from looking at the caps in
> mmc/host.h because I find the binding description obtuse, is that these
> properties (no-sdio and no-mmc) block the use of commands that would
> cause a device to malfunction. They don't appear to be required at all just
> because the board layout doesn't support these types of devices.
> 
> Conor.

Hal and/or Minda (from StarFive) any comment about this? I would ask
William but they are not involved anymore. Can we drop some of these
suspicious mmc properties, what are the reasons for these?  -E

Powered by blists - more mailing lists