| Message ID | 20240717-parrot-malt-83cc04bf6b36@spud (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Removal of non-existent DAC nodes | expand |
On 17/07/2024 11:37, Conor Dooley wrote: > From: Conor Dooley <conor.dooley@microchip.com> > > The Rohm dh2228fv (really the bh2228fv, the compatible in the kernel has > a typo) does not support frequencies above 10 MHz, nor per the > datasheet appear to use either CPOL or CPHA. I suspect that this > devicetree is abusing the compatible in order to bind the spidev driver > in Linux. Pretending to have devices on a board for this purpose is not > acceptable, so remove it. > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > --- > I could not find any documentation for this board online, and it does > not blatantly say that the device is a "spidev" like other [ab]users, so > it is possible there's actually a DAC here - but I doubt it is a > bh2228fv given the other incompatibilities. Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
adding Otavio, Am Mittwoch, 17. Juli 2024, 11:37:54 CEST schrieb Conor Dooley: > From: Conor Dooley <conor.dooley@microchip.com> > > The Rohm dh2228fv (really the bh2228fv, the compatible in the kernel has > a typo) does not support frequencies above 10 MHz, nor per the > datasheet appear to use either CPOL or CPHA. I suspect that this > devicetree is abusing the compatible in order to bind the spidev driver > in Linux. Pretending to have devices on a board for this purpose is not > acceptable, so remove it. Reasoning is sound, so I'll pick this up after the merge window. > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > --- > I could not find any documentation for this board online, and it does > not blatantly say that the device is a "spidev" like other [ab]users, so > it is possible there's actually a DAC here - but I doubt it is a > bh2228fv given the other incompatibilities. Otavio, as the original submitter of the Elgin R1 [0], do you happen to know what type of device this is? Especially as there really do not seem to be any schematics around for that board. Heiko [0] https://patchwork.kernel.org/project/linux-rockchip/patch/20190104014023.17973-4-otavio@ossystems.com.br/ > --- > arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts b/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts > index 2d9994379eb2..9df1cef406c5 100644 > --- a/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts > +++ b/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts > @@ -167,14 +167,6 @@ &spi { > pinctrl-names = "default"; > pinctrl-0 = <&spim1_clk &spim1_cs0 &spim1_tx &spim1_rx>; > status = "okay"; > - > - dh2228fv: dac@0 { > - compatible = "rohm,dh2228fv"; > - reg = <0>; > - spi-max-frequency = <24000000>; > - spi-cpha; > - spi-cpol; > - }; > }; > > &u2phy { >
On Wed, Jul 17, 2024 at 11:38 AM Otavio Salvador
<otavio.salvador@ossystems.com.br> wrote:
> @Fabio Estevam can you check this patch?
Correct, the rv1108-elgin-r1.dts board does not contain the DAC.
There is an LCD controlled via spidev though.
Conor,
If spidev is needed, what is the recommended way to describe it in the
devicetree?
Thanks
On Wed, Jul 17, 2024 at 12:34:03PM -0300, Fabio Estevam wrote: > On Wed, Jul 17, 2024 at 11:38 AM Otavio Salvador > <otavio.salvador@ossystems.com.br> wrote: > > > @Fabio Estevam can you check this patch? > > Correct, the rv1108-elgin-r1.dts board does not contain the DAC. > > There is an LCD controlled via spidev though. > > Conor, > > If spidev is needed, what is the recommended way to describe it in the > devicetree? Describe the device you actually have in a binding, and add that compatible to the spidev driver. In this case, given there's a particular LCD there, that'll work. There's nothing wrong with using spidev, it's just lying about what hardware that is there that's problematic. Otherwise, when it is an open connector where you can put anything, it's a wee bit messier... A connector binding is required then, something like the WIP mikrobus stuff that the beagle lads are working on. That's far from ready, at least on the devicetree side of things, though so you need to apply an overlay in your bootloader in that case.
On Wed, Jul 17, 2024 at 6:38 AM Conor Dooley <conor@kernel.org> wrote: > > From: Conor Dooley <conor.dooley@microchip.com> > > The Rohm dh2228fv (really the bh2228fv, the compatible in the kernel has > a typo) does not support frequencies above 10 MHz, nor per the > datasheet appear to use either CPOL or CPHA. I suspect that this > devicetree is abusing the compatible in order to bind the spidev driver > in Linux. Pretending to have devices on a board for this purpose is not > acceptable, so remove it. In the Subject: s/unlikly/unlikely Reviewed-by: Fabio Estevam <festevam@gmail.com>
Hello Conor, On 2024-07-17 11:37, Conor Dooley wrote: > From: Conor Dooley <conor.dooley@microchip.com> > > The Rohm dh2228fv (really the bh2228fv, the compatible in the kernel > has > a typo) does not support frequencies above 10 MHz, nor per the > datasheet appear to use either CPOL or CPHA. I suspect that this > devicetree is abusing the compatible in order to bind the spidev driver > in Linux. Pretending to have devices on a board for this purpose is not > acceptable, so remove it. > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> There's a small typo in the patch subject: s/unlikly/unlikely/ > --- > I could not find any documentation for this board online, and it does > not blatantly say that the device is a "spidev" like other [ab]users, > so > it is possible there's actually a DAC here - but I doubt it is a > bh2228fv given the other incompatibilities. > --- > arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts > b/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts > index 2d9994379eb2..9df1cef406c5 100644 > --- a/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts > +++ b/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts > @@ -167,14 +167,6 @@ &spi { > pinctrl-names = "default"; > pinctrl-0 = <&spim1_clk &spim1_cs0 &spim1_tx &spim1_rx>; > status = "okay"; > - > - dh2228fv: dac@0 { > - compatible = "rohm,dh2228fv"; > - reg = <0>; > - spi-max-frequency = <24000000>; > - spi-cpha; > - spi-cpol; > - }; > }; > > &u2phy {
On 2024-07-18 06:20, Dragan Simic wrote: > Hello Conor, > > On 2024-07-17 11:37, Conor Dooley wrote: >> From: Conor Dooley <conor.dooley@microchip.com> >> >> The Rohm dh2228fv (really the bh2228fv, the compatible in the kernel >> has >> a typo) does not support frequencies above 10 MHz, nor per the >> datasheet appear to use either CPOL or CPHA. I suspect that this >> devicetree is abusing the compatible in order to bind the spidev >> driver >> in Linux. Pretending to have devices on a board for this purpose is >> not >> acceptable, so remove it. >> >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > > There's a small typo in the patch subject: > s/unlikly/unlikely/ Ah, sorry for the noise, I see now it's been pointed out already. >> --- >> I could not find any documentation for this board online, and it does >> not blatantly say that the device is a "spidev" like other [ab]users, >> so >> it is possible there's actually a DAC here - but I doubt it is a >> bh2228fv given the other incompatibilities. >> --- >> arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts | 8 -------- >> 1 file changed, 8 deletions(-) >> >> diff --git a/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts >> b/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts >> index 2d9994379eb2..9df1cef406c5 100644 >> --- a/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts >> +++ b/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts >> @@ -167,14 +167,6 @@ &spi { >> pinctrl-names = "default"; >> pinctrl-0 = <&spim1_clk &spim1_cs0 &spim1_tx &spim1_rx>; >> status = "okay"; >> - >> - dh2228fv: dac@0 { >> - compatible = "rohm,dh2228fv"; >> - reg = <0>; >> - spi-max-frequency = <24000000>; >> - spi-cpha; >> - spi-cpol; >> - }; >> }; >> >> &u2phy { > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
diff --git a/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts b/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts index 2d9994379eb2..9df1cef406c5 100644 --- a/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts +++ b/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dts @@ -167,14 +167,6 @@ &spi { pinctrl-names = "default"; pinctrl-0 = <&spim1_clk &spim1_cs0 &spim1_tx &spim1_rx>; status = "okay"; - - dh2228fv: dac@0 { - compatible = "rohm,dh2228fv"; - reg = <0>; - spi-max-frequency = <24000000>; - spi-cpha; - spi-cpol; - }; }; &u2phy {