linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] riscv: dts: starfive: jh7110-common: drop no-sdio property from mmc1
@ 2025-07-24  7:55 E Shattow
  2025-07-24 16:51 ` Conor Dooley
  0 siblings, 1 reply; 7+ messages in thread
From: E Shattow @ 2025-07-24  7:55 UTC (permalink / raw)
  To: Conor Dooley, Emil Renner Berthing, Rob Herring,
	Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Alexandre Ghiti
  Cc: E Shattow, linux-riscv, devicetree, linux-kernel

Drop no-sdio property avoids a delete-property on variant board dts
having an SDIO wireless module connected to mmc1.

Signed-off-by: E Shattow <e@freeshell.de>
---
 arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
index 2eaf01775ef5..a315113840e5 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
@@ -299,7 +299,6 @@ &mmc1 {
 	assigned-clock-rates = <50000000>;
 	bus-width = <4>;
 	bootph-pre-ram;
-	no-sdio;
 	no-mmc;
 	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
 	disable-wp;

base-commit: 28fa0dcb571ab8f3be4d919f0e20e01d4e44bcb1
-- 
2.50.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: dts: starfive: jh7110-common: drop no-sdio property from mmc1
  2025-07-24  7:55 [PATCH] riscv: dts: starfive: jh7110-common: drop no-sdio property from mmc1 E Shattow
@ 2025-07-24 16:51 ` Conor Dooley
  2025-07-25  5:13   ` E Shattow
  0 siblings, 1 reply; 7+ messages in thread
From: Conor Dooley @ 2025-07-24 16:51 UTC (permalink / raw)
  To: E Shattow
  Cc: Emil Renner Berthing, Rob Herring, Krzysztof Kozlowski,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	linux-riscv, devicetree, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]

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?
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?

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?

Cheers,
Conor.

> 
> Signed-off-by: E Shattow <e@freeshell.de>
> ---
>  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> index 2eaf01775ef5..a315113840e5 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> @@ -299,7 +299,6 @@ &mmc1 {
>  	assigned-clock-rates = <50000000>;
>  	bus-width = <4>;
>  	bootph-pre-ram;
> -	no-sdio;
>  	no-mmc;
>  	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
>  	disable-wp;
> 
> base-commit: 28fa0dcb571ab8f3be4d919f0e20e01d4e44bcb1
> -- 
> 2.50.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: dts: starfive: jh7110-common: drop no-sdio property from mmc1
  2025-07-24 16:51 ` Conor Dooley
@ 2025-07-25  5:13   ` E Shattow
  2025-07-25 18:10     ` Conor Dooley
  0 siblings, 1 reply; 7+ messages in thread
From: E Shattow @ 2025-07-25  5:13 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Emil Renner Berthing, Rob Herring, Krzysztof Kozlowski,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	William Qiu, linux-riscv, devicetree, linux-kernel

Hi Conor,

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
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.

> 
> 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 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.

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?

> 
> Cheers,
> Conor.

Thank you for reviewing! -E

> 
>>
>> Signed-off-by: E Shattow <e@freeshell.de>
>> ---
>>  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> index 2eaf01775ef5..a315113840e5 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> @@ -299,7 +299,6 @@ &mmc1 {
>>  	assigned-clock-rates = <50000000>;
>>  	bus-width = <4>;
>>  	bootph-pre-ram;
>> -	no-sdio;
>>  	no-mmc;
>>  	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
>>  	disable-wp;
>>
>> base-commit: 28fa0dcb571ab8f3be4d919f0e20e01d4e44bcb1
>> -- 
>> 2.50.0
>>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: dts: starfive: jh7110-common: drop no-sdio property from mmc1
  2025-07-25  5:13   ` E Shattow
@ 2025-07-25 18:10     ` Conor Dooley
  2025-07-28  3:52       ` E Shattow
  0 siblings, 1 reply; 7+ messages in thread
From: Conor Dooley @ 2025-07-25 18:10 UTC (permalink / raw)
  To: E Shattow
  Cc: Emil Renner Berthing, Rob Herring, Krzysztof Kozlowski,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	William Qiu, linux-riscv, devicetree, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 4567 bytes --]

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.

> 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.

> 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: dts: starfive: jh7110-common: drop no-sdio property from mmc1
  2025-07-25 18:10     ` Conor Dooley
@ 2025-07-28  3:52       ` E Shattow
  2025-07-28 19:36         ` Conor Dooley
  2025-07-28 19:46         ` Conor Dooley
  0 siblings, 2 replies; 7+ messages in thread
From: E Shattow @ 2025-07-28  3:52 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Emil Renner Berthing, Rob Herring, Krzysztof Kozlowski,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	William Qiu, linux-riscv, devicetree, linux-kernel, Hal Feng,
	Minda Chen

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: dts: starfive: jh7110-common: drop no-sdio property from mmc1
  2025-07-28  3:52       ` E Shattow
@ 2025-07-28 19:36         ` Conor Dooley
  2025-07-28 19:46         ` Conor Dooley
  1 sibling, 0 replies; 7+ messages in thread
From: Conor Dooley @ 2025-07-28 19:36 UTC (permalink / raw)
  To: E Shattow
  Cc: Emil Renner Berthing, Rob Herring, Krzysztof Kozlowski,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	William Qiu, linux-riscv, devicetree, linux-kernel, Hal Feng,
	Minda Chen

[-- Attachment #1: Type: text/plain, Size: 1397 bytes --]

On Sun, Jul 27, 2025 at 08:52:50PM -0700, E Shattow wrote:

> >> 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.

Ye, moved out to the board dts is my preference over using /delete-property/.
Things that aren't actually common shouldn't be in the common file. I do
agree that in this case, removal entirely is probably the right call.
There's some sdio wifi cards that can go into sd card slots, so just
because the mmc controller is connected to a sd card slot doesn't mean
no-sdio should be used.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: dts: starfive: jh7110-common: drop no-sdio property from mmc1
  2025-07-28  3:52       ` E Shattow
  2025-07-28 19:36         ` Conor Dooley
@ 2025-07-28 19:46         ` Conor Dooley
  1 sibling, 0 replies; 7+ messages in thread
From: Conor Dooley @ 2025-07-28 19:46 UTC (permalink / raw)
  To: E Shattow
  Cc: Emil Renner Berthing, Rob Herring, Krzysztof Kozlowski,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	William Qiu, linux-riscv, devicetree, linux-kernel, Hal Feng,
	Minda Chen

[-- Attachment #1: Type: text/plain, Size: 400 bytes --]

On Sun, Jul 27, 2025 at 08:52:50PM -0700, E Shattow wrote:
> 
> 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?

Additionally, William is mentioned in several maintainers entries, could
you StarFive folks either replace/remove him please?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-07-28 19:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-24  7:55 [PATCH] riscv: dts: starfive: jh7110-common: drop no-sdio property from mmc1 E Shattow
2025-07-24 16:51 ` Conor Dooley
2025-07-25  5:13   ` E Shattow
2025-07-25 18:10     ` Conor Dooley
2025-07-28  3:52       ` E Shattow
2025-07-28 19:36         ` Conor Dooley
2025-07-28 19:46         ` Conor Dooley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).