| Message ID | 20221220192854.521647-1-bmasney@redhat.com |
|---|---|
| Headers | show |
| Series | dts: qcom: sc8280xp: add i2c, spi, and rng nodes | expand |
On 20.12.2022 20:28, Brian Masney wrote: > Add the necessary device tree node for qcom,prng-ee so we can use the > hardware random number generator. This functionality was tested on a > SA8540p automotive development board using kcapi-rng from libkcapi. > > Signed-off-by: Brian Masney <bmasney@redhat.com> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > Changes from v2 to v3: > - Correctly sort node by MMIO address > > Patch introduced in v2 > > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index 4591d411f5fb..6c2cae83dac6 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -1602,6 +1602,13 @@ spi15: spi@a9c000 { > }; > }; > > + rng: rng@10d3000 { > + compatible = "qcom,prng-ee"; > + reg = <0 0x010d3000 0 0x1000>; > + clocks = <&rpmhcc RPMH_HWKM_CLK>; > + clock-names = "core"; > + }; > + > pcie4: pcie@1c00000 { > device_type = "pci"; > compatible = "qcom,pcie-sc8280xp";
On Tue, Dec 20, 2022 at 02:28:49PM -0500, Brian Masney wrote: > In preparation for adding the missing SPI and I2C nodes to > sc8280xp.dtsi, it was decided to rename all of the existing qupX_ > uart, spi, and i2c nodes to drop the qupX_ prefix. Let's go ahead > and rename qup2_i2c5 to i2c21. Under the old name, this was the 5th > index under qup2, which starts at index 16. > > Note that some nodes are moved in the file by this patch to preserve > the expected sort order in the file. Additionally, the properties > within the pinctrl state node are sorted to match the expected order > that's typically done in other DTs. > > Signed-off-by: Brian Masney <bmasney@redhat.com> > Link: https://lore.kernel.org/lkml/20221212182314.1902632-1-bmasney@redhat.com/ > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
On Tue, Dec 20, 2022 at 02:28:51PM -0500, Brian Masney wrote: > Add the missing nodes for the i2c buses that's present on this SoC. > > This work was derived from various patches that Qualcomm delivered > to Red Hat in a downstream kernel. > > Signed-off-by: Brian Masney <bmasney@redhat.com> > --- > Changes from v2 to v3 > - None > > Changes from v1 to v2 > - Dropped qupX_ prefix from labels. (Johan) > > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 352 +++++++++++++++++++++++++ > 1 file changed, 352 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index f1111cd7f679..a502d4e19d98 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -813,6 +813,38 @@ qup2: geniqup@8c0000 { > > status = "disabled"; > > + i2c16: i2c@880000 { > + compatible = "qcom,geni-i2c"; > + reg = <0 0x00880000 0 0x4000>; > + clocks = <&gcc GCC_QUPV3_WRAP2_S0_CLK>; > + clock-names = "se"; > + interrupts = <GIC_SPI 373 IRQ_TYPE_LEVEL_HIGH>; > + #address-cells = <1>; > + #size-cells = <0>; I'm aware that the two current i2c nodes has these two properties here in the middle, but would you mind moving '#address-cells' and '#size-cells' after 'reg' instead where I'd expect them to be? Same for the spi patch. I can clean up the existing two nodes (and binding example) unless you want to do it. > + power-domains = <&rpmhpd SC8280XP_CX>; > + interconnects = <&clk_virt MASTER_QUP_CORE_2 0 &clk_virt SLAVE_QUP_CORE_2 0>, > + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_QUP_2 0>, > + <&aggre1_noc MASTER_QUP_2 0 &mc_virt SLAVE_EBI1 0>; > + interconnect-names = "qup-core", "qup-config", "qup-memory"; > + status = "disabled"; > + }; Johan
On Fri, Dec 23, 2022 at 11:37:02AM +0100, Johan Hovold wrote: > On Tue, Dec 20, 2022 at 02:28:51PM -0500, Brian Masney wrote: > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > index f1111cd7f679..a502d4e19d98 100644 > > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > @@ -813,6 +813,38 @@ qup2: geniqup@8c0000 { > > > > status = "disabled"; > > > > + i2c16: i2c@880000 { > > + compatible = "qcom,geni-i2c"; > > + reg = <0 0x00880000 0 0x4000>; > > + clocks = <&gcc GCC_QUPV3_WRAP2_S0_CLK>; > > + clock-names = "se"; > > + interrupts = <GIC_SPI 373 IRQ_TYPE_LEVEL_HIGH>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > I'm aware that the two current i2c nodes has these two properties here > in the middle, but would you mind moving '#address-cells' and > '#size-cells' after 'reg' instead where I'd expect them to be? > > Same for the spi patch. > > I can clean up the existing two nodes (and binding example) unless you > want to do it. I'll clean up the existing nodes, qcom,i2c-geni-qcom.yaml, and qcom,geni-se.yaml in my next version. Brian
On 23.12.2022 11:37, Johan Hovold wrote: > On Tue, Dec 20, 2022 at 02:28:51PM -0500, Brian Masney wrote: >> Add the missing nodes for the i2c buses that's present on this SoC. >> >> This work was derived from various patches that Qualcomm delivered >> to Red Hat in a downstream kernel. >> >> Signed-off-by: Brian Masney <bmasney@redhat.com> >> --- >> Changes from v2 to v3 >> - None >> >> Changes from v1 to v2 >> - Dropped qupX_ prefix from labels. (Johan) >> >> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 352 +++++++++++++++++++++++++ >> 1 file changed, 352 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> index f1111cd7f679..a502d4e19d98 100644 >> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> @@ -813,6 +813,38 @@ qup2: geniqup@8c0000 { >> >> status = "disabled"; >> >> + i2c16: i2c@880000 { >> + compatible = "qcom,geni-i2c"; >> + reg = <0 0x00880000 0 0x4000>; >> + clocks = <&gcc GCC_QUPV3_WRAP2_S0_CLK>; >> + clock-names = "se"; >> + interrupts = <GIC_SPI 373 IRQ_TYPE_LEVEL_HIGH>; >> + #address-cells = <1>; >> + #size-cells = <0>; > > I'm aware that the two current i2c nodes has these two properties here > in the middle, but would you mind moving '#address-cells' and > '#size-cells' after 'reg' instead where I'd expect them to be? Hm.. we've been sticking them somewhere near the end for the longest time for every bus-like, or generally "i have childen" type node.. I see it's a rather mixed bag in non-qcom SoCs, people just seem to put it wherever they please.. The dt spec doesn't seem to mention any preference fwiw. Konrad > > Same for the spi patch. > > I can clean up the existing two nodes (and binding example) unless you > want to do it. > >> + power-domains = <&rpmhpd SC8280XP_CX>; >> + interconnects = <&clk_virt MASTER_QUP_CORE_2 0 &clk_virt SLAVE_QUP_CORE_2 0>, >> + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_QUP_2 0>, >> + <&aggre1_noc MASTER_QUP_2 0 &mc_virt SLAVE_EBI1 0>; >> + interconnect-names = "qup-core", "qup-config", "qup-memory"; >> + status = "disabled"; >> + }; > > Johan
On Fri, Dec 23, 2022 at 01:42:32PM +0100, Konrad Dybcio wrote: > On 23.12.2022 11:37, Johan Hovold wrote: > >> + i2c16: i2c@880000 { > >> + compatible = "qcom,geni-i2c"; > >> + reg = <0 0x00880000 0 0x4000>; > >> + clocks = <&gcc GCC_QUPV3_WRAP2_S0_CLK>; > >> + clock-names = "se"; > >> + interrupts = <GIC_SPI 373 IRQ_TYPE_LEVEL_HIGH>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > > > > I'm aware that the two current i2c nodes has these two properties here > > in the middle, but would you mind moving '#address-cells' and > > '#size-cells' after 'reg' instead where I'd expect them to be? > Hm.. we've been sticking them somewhere near the end for the longest > time for every bus-like, or generally "i have childen" type node.. > I see it's a rather mixed bag in non-qcom SoCs, people just seem to > put it wherever they please.. The dt spec doesn't seem to mention any > preference fwiw. The rationale for placing them under 'reg' is that you keep the address-related properties together (e.g. 'reg', '#address-cells', '#size-cells' and 'ranges'). Johan
On 23.12.2022 14:05, Johan Hovold wrote: > On Fri, Dec 23, 2022 at 01:42:32PM +0100, Konrad Dybcio wrote: >> On 23.12.2022 11:37, Johan Hovold wrote: > >>>> + i2c16: i2c@880000 { >>>> + compatible = "qcom,geni-i2c"; >>>> + reg = <0 0x00880000 0 0x4000>; >>>> + clocks = <&gcc GCC_QUPV3_WRAP2_S0_CLK>; >>>> + clock-names = "se"; >>>> + interrupts = <GIC_SPI 373 IRQ_TYPE_LEVEL_HIGH>; >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>> >>> I'm aware that the two current i2c nodes has these two properties here >>> in the middle, but would you mind moving '#address-cells' and >>> '#size-cells' after 'reg' instead where I'd expect them to be? > >> Hm.. we've been sticking them somewhere near the end for the longest >> time for every bus-like, or generally "i have childen" type node.. >> I see it's a rather mixed bag in non-qcom SoCs, people just seem to >> put it wherever they please.. The dt spec doesn't seem to mention any >> preference fwiw. > > The rationale for placing them under 'reg' is that you keep the > address-related properties together (e.g. 'reg', '#address-cells', > '#size-cells' and 'ranges'). Okay, I see the point. Konrad > > Johan
On Wed, Dec 21, 2022 at 01:41:52PM -0600, Steev Klimaszewski wrote: > One note, and this isn't due to your patches at all, but the touchscreen on > the Thinkpad X13s needs to be manually bound in order to work via echo > 1-0010 | sudo tee /sys/bus/i2c/drivers/i2c_hid_of/bind - this patch does not > affect that, though I had hoped maybe it would. > > Tested on the Lenovo Thinkpad X13s > > Tested-by: Steev Klimaszewski <steev@kali.org> Hi Steev, I believe that I remember reading at some point that the touchscreen issue on the x13s was related to some probe deferral issues. If so, try adding this patch series from Javier to see if that helps the situation: https://lore.kernel.org/lkml/20221116115348.517599-1-javierm@redhat.com/ Javier separately encountered a probe deferral issue when enabling a Snapdragon-based Chromebook on Fedora that caused him to work on that patch series. Brian