Devicetree
 help / color / mirror / Atom feed
From: Conor Dooley <conor@kernel.org>
To: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Cc: Conor Dooley <conor.dooley@microchip.com>,
	linux-riscv@lists.infradead.org,
	Emil Renner Berthing <kernel@esmil.dk>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	William Qiu <william.qiu@starfivetech.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] riscv: dts: starfive: remove non-existant spi device from jh7110-common.dtsi
Date: Fri, 26 Jul 2024 13:46:18 +0100	[thread overview]
Message-ID: <20240726-likewise-satin-81a7a4a3885c@spud> (raw)
In-Reply-To: <CAJM55Z9FAH-uiNmXDELM0gkYjHue+g8JQgOryxOCv4OXJ9f5EA@mail.gmail.com>

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

On Fri, Jul 26, 2024 at 08:29:39AM -0400, Emil Renner Berthing wrote:
> Conor Dooley wrote:
> > There is no rohm,dh2228fv on any of supported JH7110 boards - in fact
> > the dh2228fv almost certainly does not exist as it is not a valid Rohm
> > part number. Likely a typo by Maxime when adding the device originally,
> > and should have been bh2228fv, but these boards do not have a bh2228fv
> > either! Remove it from jh7110-common.dtsi - pretending to have a device
> > so that the spidev driver will be bound by Linux is not acceptable.
> 
> This patch is correct, but as you mention the fake device was most likely added
> in order to use spidev from userspace with random devices added on the exposed
> pins. In case someone actually makes use of this wouldn't this be a regression?
> What is the right way to support this?

Unfortunately, there's no "right way" that's supported for for this
particular case. If people want to use spidev for their device, they
should either document it in the bindings, add the compatible to the
spidev driver and use an overlay to add the device to the dts or they
can r bind the spidev driver to the device from userspace.

The other thing, which doesn't exist yet, is a connector binding. The
folks are Beagle are currently working on creating a connector binding
for the Mikrobus connector - but that's rather far from complete at the
moment.

Cheers,
Conor.

> > Fixes: 74fb20c8f05d ("riscv: dts: starfive: Add spi node and pins configuration")
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> > CC: Emil Renner Berthing <kernel@esmil.dk>
> > CC: Conor Dooley <conor@kernel.org>
> > CC: Rob Herring <robh@kernel.org>
> > CC: Krzysztof Kozlowski <krzk+dt@kernel.org>
> > CC: Paul Walmsley <paul.walmsley@sifive.com>
> > CC: Palmer Dabbelt <palmer@dabbelt.com>
> > CC: Albert Ou <aou@eecs.berkeley.edu>
> > CC: William Qiu <william.qiu@starfivetech.com>
> > CC: linux-riscv@lists.infradead.org
> > CC: devicetree@vger.kernel.org
> > CC: linux-kernel@vger.kernel.org
> > ---
> >  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ------
> >  1 file changed, 6 deletions(-)
> >
> > diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> > index 8ff6ea64f048..395436ec0f97 100644
> > --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> > +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> > @@ -346,12 +346,6 @@ &spi0 {
> >  	pinctrl-names = "default";
> >  	pinctrl-0 = <&spi0_pins>;
> >  	status = "okay";
> > -
> > -	spi_dev0: spi@0 {
> > -		compatible = "rohm,dh2228fv";
> > -		reg = <0>;
> > -		spi-max-frequency = <10000000>;
> > -	};
> >  };
> >
> >  &sysgpio {
> > --
> > 2.43.2
> >

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

  reply	other threads:[~2024-07-26 12:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-16 10:54 [PATCH v1] riscv: dts: starfive: remove non-existant spi device from jh7110-common.dtsi Conor Dooley
2024-07-17  8:42 ` Krzysztof Kozlowski
2024-07-26 12:29 ` Emil Renner Berthing
2024-07-26 12:46   ` Conor Dooley [this message]
2024-07-26 12:59     ` Emil Renner Berthing
2024-08-07 17:26 ` Conor Dooley

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=20240726-likewise-satin-81a7a4a3885c@spud \
    --to=conor@kernel.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=conor.dooley@microchip.com \
    --cc=devicetree@vger.kernel.org \
    --cc=emil.renner.berthing@canonical.com \
    --cc=kernel@esmil.dk \
    --cc=krzk+dt@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=robh@kernel.org \
    --cc=william.qiu@starfivetech.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox