From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 References: <20180721192846.18811-1-martin.blumenstingl@googlemail.com> <20180721192846.18811-2-martin.blumenstingl@googlemail.com> <20180725200755.GA24555@rob-hp-laptop> In-Reply-To: <20180725200755.GA24555@rob-hp-laptop> From: Martin Blumenstingl Date: Wed, 25 Jul 2018 23:16:24 +0200 Message-ID: Subject: Re: [PATCH 1/3] dt-bindings: clock: meson8b: use the registers from the HHI syscon Content-Type: text/plain; charset="UTF-8" To: robh@kernel.org Cc: Neil Armstrong , jbrunet@baylibre.com, mark.rutland@arm.com, linux-amlogic@lists.infradead.org, devicetree@vger.kernel.org, mturquette@baylibre.com, sboyd@kernel.org, carlo@caione.org, khilman@baylibre.com, linux-clk@vger.kernel.org List-ID: Hi Rob, On Wed, Jul 25, 2018 at 10:07 PM Rob Herring wrote: > > On Sat, Jul 21, 2018 at 09:28:44PM +0200, Martin Blumenstingl wrote: > > The clock controller on Meson8/Meson8m2 and Meson8b is part of a > > register region called "HHI". This register area contains more > > functionality than just a clock controller: > > - the clock controller > > - some reset controller bits > > - temperature sensor calibration data (on Meson8b and Meson8m2 only) > > - HDMI controller > > > > The HHI register area may be accessed concurrently. Allow this by using > > a "system controller" as parent node. > > Why? A single node can be a provider of multiple things. Maybe the HDMI > should be a child since it will involve graph nodes, but the rest can be > one node. There should be numerous examples of blocks that are clock and > reset controllers. I understand that a node can provide multiple "things" - currently it's a clock controller and a reset controller the HDMI controller could also be integrated in a similar way however, I do not know how to access the temperature sensor calibration data there is an ADC - one of it's channel has access to a temperature sensor this ADC is located at CBUS offset 0x8680 and we already have a driver for it (meson-saradc) my problem is that the temperature sensor has to be calibrated - this is done by: - read data from efuse - write 4 bits of calibration data to some register in the ADC's register space - write a 5th bit of calibration data to a (seemingly random) register in the HHI register space (if one of the 5 bits is not written to it's correct location then the temperature sensor reads bogus values) I am not sure how to handle this without passing the HHI region to the meson-saradc driver and letting that initialize all the temperature calibration data bits (in it's own register space as well as the HHI register space) do you have any suggestion here? > > > > Signed-off-by: Martin Blumenstingl > > --- > > .../bindings/clock/amlogic,meson8b-clkc.txt | 13 +++++-------- > > 1 file changed, 5 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt > > index b455c5aa9139..38fb979210d3 100644 > > --- a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt > > +++ b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt > > @@ -9,15 +9,13 @@ Required Properties: > > - "amlogic,meson8-clkc" for Meson8 (S802) SoCs > > - "amlogic,meson8b-clkc" for Meson8 (S805) SoCs > > - "amlogic,meson8m2-clkc" for Meson8m2 (S812) SoCs > > -- reg: it must be composed by two tuples: > > - 0) physical base address of the xtal register and length of memory > > - mapped region. > > - 1) physical base address of the clock controller and length of memory > > - mapped region. > > - > > - #clock-cells: should be 1. > > - #reset-cells: should be 1. > > > > +Parent node should have the following properties : > > +- compatible: "syscon", "simple-mfd" > > These 2 compatibles alone are not valid. so I should add (for example) "amlogic,meson8b-hhi-sysctrl" as first compatible? so the result would be: compatible = "amlogic,meson8b-hhi-sysctrl", "syscon", "simple-mfd"; Regards Martin From mboxrd@z Thu Jan 1 00:00:00 1970 From: martin.blumenstingl@googlemail.com (Martin Blumenstingl) Date: Wed, 25 Jul 2018 23:16:24 +0200 Subject: [PATCH 1/3] dt-bindings: clock: meson8b: use the registers from the HHI syscon In-Reply-To: <20180725200755.GA24555@rob-hp-laptop> References: <20180721192846.18811-1-martin.blumenstingl@googlemail.com> <20180721192846.18811-2-martin.blumenstingl@googlemail.com> <20180725200755.GA24555@rob-hp-laptop> Message-ID: To: linus-amlogic@lists.infradead.org List-Id: linus-amlogic.lists.infradead.org Hi Rob, On Wed, Jul 25, 2018 at 10:07 PM Rob Herring wrote: > > On Sat, Jul 21, 2018 at 09:28:44PM +0200, Martin Blumenstingl wrote: > > The clock controller on Meson8/Meson8m2 and Meson8b is part of a > > register region called "HHI". This register area contains more > > functionality than just a clock controller: > > - the clock controller > > - some reset controller bits > > - temperature sensor calibration data (on Meson8b and Meson8m2 only) > > - HDMI controller > > > > The HHI register area may be accessed concurrently. Allow this by using > > a "system controller" as parent node. > > Why? A single node can be a provider of multiple things. Maybe the HDMI > should be a child since it will involve graph nodes, but the rest can be > one node. There should be numerous examples of blocks that are clock and > reset controllers. I understand that a node can provide multiple "things" - currently it's a clock controller and a reset controller the HDMI controller could also be integrated in a similar way however, I do not know how to access the temperature sensor calibration data there is an ADC - one of it's channel has access to a temperature sensor this ADC is located at CBUS offset 0x8680 and we already have a driver for it (meson-saradc) my problem is that the temperature sensor has to be calibrated - this is done by: - read data from efuse - write 4 bits of calibration data to some register in the ADC's register space - write a 5th bit of calibration data to a (seemingly random) register in the HHI register space (if one of the 5 bits is not written to it's correct location then the temperature sensor reads bogus values) I am not sure how to handle this without passing the HHI region to the meson-saradc driver and letting that initialize all the temperature calibration data bits (in it's own register space as well as the HHI register space) do you have any suggestion here? > > > > Signed-off-by: Martin Blumenstingl > > --- > > .../bindings/clock/amlogic,meson8b-clkc.txt | 13 +++++-------- > > 1 file changed, 5 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt > > index b455c5aa9139..38fb979210d3 100644 > > --- a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt > > +++ b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt > > @@ -9,15 +9,13 @@ Required Properties: > > - "amlogic,meson8-clkc" for Meson8 (S802) SoCs > > - "amlogic,meson8b-clkc" for Meson8 (S805) SoCs > > - "amlogic,meson8m2-clkc" for Meson8m2 (S812) SoCs > > -- reg: it must be composed by two tuples: > > - 0) physical base address of the xtal register and length of memory > > - mapped region. > > - 1) physical base address of the clock controller and length of memory > > - mapped region. > > - > > - #clock-cells: should be 1. > > - #reset-cells: should be 1. > > > > +Parent node should have the following properties : > > +- compatible: "syscon", "simple-mfd" > > These 2 compatibles alone are not valid. so I should add (for example) "amlogic,meson8b-hhi-sysctrl" as first compatible? so the result would be: compatible = "amlogic,meson8b-hhi-sysctrl", "syscon", "simple-mfd"; Regards Martin