Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v5 02/11] dt-bindings: i2c: Discard i2c-slave flag from the DW I2C example
From: Rob Herring @ 2020-05-29 18:13 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Jarkko Nikula, Wolfram Sang, Alexey Malahov,
	Thomas Bogendoerfer, Andy Shevchenko, Mika Westerberg, linux-mips,
	linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200527153351.rmzguymz7lm6gvsx@mobilestation>

On Wed, May 27, 2020 at 06:33:51PM +0300, Serge Semin wrote:
> Rob,
> Could you pay attention to this patch? The patchset review procedure is
> nearly over, while the DT part is only partly reviewed by you.

Pretty sure I commented on this. Not sure what version, you're sending 
new versions too fast. Give people time to review.

> 
> Thanks
> -Sergey
> 
> On Wed, May 27, 2020 at 06:30:37PM +0300, Serge Semin wrote:
> > dtc currently doesn't support I2C_OWN_SLAVE_ADDRESS flag set in the
> > i2c "reg" property. If it is the compiler will print a warning:
> > 
> > Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64: I2C bus unit address format error, expected "40000064"
> > Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64:reg: I2C address must be less than 10-bits, got "0x40000064"
> > 
> > In order to silence dtc up let's discard the flag from the DW I2C DT
> > binding example for now. Just revert this commit when dtc is fixed.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Cc: linux-mips@vger.kernel.org
> > 
> > ---
> > 
> > Changelog v3:
> > - This is a new patch created as a result of the Rob request to remove
> >   the EEPROM-slave bit setting in the DT binndings example until the dtc
> >   is fixed.
> > ---
> >  Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> > index 4bd430b2b41d..101d78e8f19d 100644
> > --- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> > +++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> > @@ -137,7 +137,7 @@ examples:
> >  
> >        eeprom@64 {
> >          compatible = "linux,slave-24c02";
> > -        reg = <0x40000064>;
> > +        reg = <0x64>;

This is wrong though because "linux,slave-24c02" should have bit 30 set. 
(And either the unit-address was wrong or we can define the unit-address 
does not include the high bits.)

Rob

^ permalink raw reply

* Re: [PATCH v3 02/25] dt-bindings: clock: Add a binding for the RPi Firmware clocks
From: Rob Herring @ 2020-05-29 18:14 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: linux-kernel, Tim Gover, Rob Herring, Stephen Boyd,
	Dave Stevenson, Michael Turquette, devicetree, linux-arm-kernel,
	Nicolas Saenz Julienne, linux-clk, Phil Elwell,
	bcm-kernel-feedback-list, linux-rpi-kernel
In-Reply-To: <919e2f2f13583d4d53d0e95b81fc3fb8a7606107.1590594293.git-series.maxime@cerno.tech>

On Wed, 27 May 2020 17:44:58 +0200, Maxime Ripard wrote:
> The firmware running on the RPi VideoCore can be used to discover and
> change the various clocks running in the BCM2711. Since devices will
> need to use them through the DT, let's add a pretty simple binding.
> 
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-clk@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v3 104/105] dt-bindings: display: vc4: hdmi: Add BCM2711 HDMI controllers bindings
From: Rob Herring @ 2020-05-29 18:18 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Nicolas Saenz Julienne, Eric Anholt, dri-devel, linux-rpi-kernel,
	bcm-kernel-feedback-list, linux-arm-kernel, linux-kernel,
	Dave Stevenson, Tim Gover, Phil Elwell, devicetree
In-Reply-To: <e85e24a494a3ff41177c94673ced0f4280b6a0ee.1590594512.git-series.maxime@cerno.tech>

On Wed, May 27, 2020 at 05:49:14PM +0200, Maxime Ripard wrote:
> The HDMI controllers found in the BCM2711 SoC need some adjustments to the
> bindings, especially since the registers have been shuffled around in more
> register ranges.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 109 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> new file mode 100644
> index 000000000000..6091fe3d315b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license...

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/brcm,bcm2711-hdmi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom BCM2711 HDMI Controller Device Tree Bindings
> +
> +maintainers:
> +  - Eric Anholt <eric@anholt.net>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - brcm,bcm2711-hdmi0
> +      - brcm,bcm2711-hdmi1

What's the difference between the 2 blocks? 

> +
> +  reg:
> +    items:
> +      - description: HDMI controller register range
> +      - description: DVP register range
> +      - description: HDMI PHY register range
> +      - description: Rate Manager register range
> +      - description: Packet RAM register range
> +      - description: Metadata RAM register range
> +      - description: CSC register range
> +      - description: CEC register range
> +      - description: HD register range
> +
> +  reg-names:
> +    items:
> +      - const: hdmi
> +      - const: dvp
> +      - const: phy
> +      - const: rm
> +      - const: packet
> +      - const: metadata
> +      - const: csc
> +      - const: cec
> +      - const: hd
> +
> +  clocks:
> +    description: The HDMI state machine clock
> +
> +  clock-names:
> +    const: hdmi
> +
> +  ddc:
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/phandle
> +    description: >
> +      Phandle of the I2C controller used for DDC EDID probing

Goes in the connector.

And isn't the standard name ddc-i2c-bus?

> +
> +  hpd-gpios:
> +    description: >
> +      The GPIO pin for the HDMI hotplug detect (if it doesn't appear
> +      as an interrupt/status bit in the HDMI controller itself)

Goes in the connector.

> +
> +  dmas:
> +    maxItems: 1
> +    description: >
> +      Should contain one entry pointing to the DMA channel used to
> +      transfer audio data.
> +
> +  dma-names:
> +    const: audio-rx
> +
> +  resets:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - resets
> +  - ddc
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    hdmi0: hdmi@7ef00700 {
> +        compatible = "brcm,bcm2711-hdmi0";
> +        reg = <0x7ef00700 0x300>,
> +              <0x7ef00300 0x200>,
> +              <0x7ef00f00 0x80>,
> +              <0x7ef00f80 0x80>,
> +              <0x7ef01b00 0x200>,
> +              <0x7ef01f00 0x400>,
> +              <0x7ef00200 0x80>,
> +              <0x7ef04300 0x100>,
> +              <0x7ef20000 0x100>;
> +        reg-names = "hdmi",
> +                    "dvp",
> +                    "phy",
> +                    "rm",
> +                    "packet",
> +                    "metadata",
> +                    "csc",
> +                    "cec",
> +                    "hd";
> +        clocks = <&firmware_clocks 13>;
> +        clock-names = "hdmi";
> +        resets = <&dvp 0>;
> +        ddc = <&ddc0>;
> +    };
> +
> +...
> -- 
> git-series 0.9.1

^ permalink raw reply

* Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
From: Martin Kepplinger @ 2020-05-29 18:20 UTC (permalink / raw)
  To: Pavel Machek
  Cc: robh, kernel, shawnguo, s.hauer, kernel, festevam, linux-imx,
	mchehab, Anson.Huang, agx, angus, linux-kernel, devicetree,
	linux-arm-kernel
In-Reply-To: <20200529180743.GA1081@bug>

On 29.05.20 20:07, Pavel Machek wrote:
> Hi!
> 
> Plus, do we need calibration matrix for magnetometer?

I guess so. It's not a calibration matrix, it's the mount matrix that
tells you how the chip is placed on the PCB relative to a "natural"
orientation, see
https://github.com/torvalds/linux/blob/master/Documentation/ABI/testing/sysfs-bus-iio#L1638

> 
> Best regards,
> 								Pavel
> 

^ permalink raw reply

* Re: [PATCH v5 02/11] dt-bindings: i2c: Discard i2c-slave flag from the DW I2C example
From: Serge Semin @ 2020-05-29 18:22 UTC (permalink / raw)
  To: Rob Herring
  Cc: Serge Semin, Jarkko Nikula, Wolfram Sang, Alexey Malahov,
	Thomas Bogendoerfer, Andy Shevchenko, Mika Westerberg, linux-mips,
	linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200529181338.GA2676189@bogus>

On Fri, May 29, 2020 at 12:13:38PM -0600, Rob Herring wrote:
> On Wed, May 27, 2020 at 06:33:51PM +0300, Serge Semin wrote:
> > Rob,
> > Could you pay attention to this patch? The patchset review procedure is
> > nearly over, while the DT part is only partly reviewed by you.
> 

> Pretty sure I commented on this. Not sure what version, you're sending 
> new versions too fast. Give people time to review.

Yeah, you did. Sorry for sending the new versions very fast. Normally I prefer
to keep up with comments so to past a particular maintainer review as fast as
possible without long delays. In my experience the longer you wait, the lesser
maintainers remember about your patchset, the harder for one to continue the
next versions review.

Regarding this patch the brand new version on is v6:
[PATCH v6 02/11] dt-bindings: i2c: Convert DW I2C slave to the DW I2C master example

Could you please find it in your email log? I've left a note there for you about
options what we can do with this case.

-Sergey

> 
> > 
> > Thanks
> > -Sergey
> > 
> > On Wed, May 27, 2020 at 06:30:37PM +0300, Serge Semin wrote:
> > > dtc currently doesn't support I2C_OWN_SLAVE_ADDRESS flag set in the
> > > i2c "reg" property. If it is the compiler will print a warning:
> > > 
> > > Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64: I2C bus unit address format error, expected "40000064"
> > > Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64:reg: I2C address must be less than 10-bits, got "0x40000064"
> > > 
> > > In order to silence dtc up let's discard the flag from the DW I2C DT
> > > binding example for now. Just revert this commit when dtc is fixed.
> > > 
> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > Cc: linux-mips@vger.kernel.org
> > > 
> > > ---
> > > 
> > > Changelog v3:
> > > - This is a new patch created as a result of the Rob request to remove
> > >   the EEPROM-slave bit setting in the DT binndings example until the dtc
> > >   is fixed.
> > > ---
> > >  Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> > > index 4bd430b2b41d..101d78e8f19d 100644
> > > --- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> > > +++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> > > @@ -137,7 +137,7 @@ examples:
> > >  
> > >        eeprom@64 {
> > >          compatible = "linux,slave-24c02";
> > > -        reg = <0x40000064>;
> > > +        reg = <0x64>;
> 
> This is wrong though because "linux,slave-24c02" should have bit 30 set. 
> (And either the unit-address was wrong or we can define the unit-address 
> does not include the high bits.)
> 
> Rob

^ permalink raw reply

* Re: [PATCH v4 0/2] soc: ti: add k3 platforms chipid module driver
From: Grygorii Strashko @ 2020-05-29 18:22 UTC (permalink / raw)
  To: Santosh Shilimkar, Tero Kristo, Rob Herring, Lokesh Vutla,
	devicetree, Arnd Bergmann
  Cc: Dave Gerlach, Sekhar Nori, linux-arm-kernel, linux-kernel,
	Nishanth Menon
In-Reply-To: <20200512123449.16517-1-grygorii.strashko@ti.com>

Hi Santosh,

On 12/05/2020 15:34, Grygorii Strashko wrote:
> Hi All,
> 
> This series introduces TI K3 Multicore SoC platforms chipid module driver
> which provides identification support of the TI K3 SoCs (family, revision)
> and register this information with the SoC bus. It is available under
> /sys/devices/soc0/ for user space, and can be checked, where needed,
> in Kernel using soc_device_match().
> It is also required for introducing support for new revisions of
> K3 AM65x/J721E SoCs.
> 
> Example J721E:
>    # cat /sys/devices/soc0/{machine,family,revision}
>    Texas Instruments K3 J721E SoC
>    J721E
>    SR1.0
> 
> Example AM65x:
>    # cat /sys/devices/soc0/{machine,family,revision}
>    Texas Instruments AM654 Base Board
>    AM65X
>    SR1.0
> 
> Changes in v4:
>   - convert to platform_driver as suggested by Arnd Bergmann <arnd@arndb.de>
> 
> Changes in v3:
>   - add handling of kasprintf() fail
> 
> Changes in v2:
>   - pr_debug() replaced with pr_info() to show SoC info on init
>   - minor format change
>   - split series on driver and platform changes
>   - add Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>
> 
> v3: https://lkml.org/lkml/2020/5/8/357
> v2: https://lkml.org/lkml/2020/5/5/1193
> v1: https://lwn.net/Articles/818577/
> 
> Grygorii Strashko (2):
>    dt-bindings: soc: ti: add binding for k3 platforms chipid module
>    soc: ti: add k3 platforms chipid module driver
> 
>   .../bindings/soc/ti/k3-socinfo.yaml           |  40 +++++
>   drivers/soc/ti/Kconfig                        |  10 ++
>   drivers/soc/ti/Makefile                       |   1 +
>   drivers/soc/ti/k3-socinfo.c                   | 152 ++++++++++++++++++
>   4 files changed, 203 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/soc/ti/k3-socinfo.yaml
>   create mode 100644 drivers/soc/ti/k3-socinfo.c
> 

Any chances you can pick this up?

-- 
Best regards,
grygorii

^ permalink raw reply

* Re: [PATCH net-next v4 1/4] dt-bindings: net: Add tx and rx internal delays
From: Rob Herring @ 2020-05-29 18:25 UTC (permalink / raw)
  To: Dan Murphy
  Cc: andrew, f.fainelli, hkallweit1, davem, netdev, linux-kernel,
	devicetree
In-Reply-To: <20200527164934.28651-2-dmurphy@ti.com>

On Wed, May 27, 2020 at 11:49:31AM -0500, Dan Murphy wrote:
> tx-internal-delays and rx-internal-delays are a common setting for RGMII
> capable devices.
> 
> These properties are used when the phy-mode or phy-controller is set to
> rgmii-id, rgmii-rxid or rgmii-txid.  These modes indicate to the
> controller that the PHY will add the internal delay for the connection.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  .../bindings/net/ethernet-controller.yaml          | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> index ac471b60ed6a..70702a4ef5e8 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> @@ -143,6 +143,20 @@ properties:
>        Specifies the PHY management type. If auto is set and fixed-link
>        is not specified, it uses MDIO for management.
>  
> +  rx-internal-delay-ps:
> +    $ref: /schemas/types.yaml#definitions/uint32
> +    description: |
> +      RGMII Receive PHY Clock Delay defined in pico seconds.  This is used for
> +      PHY's that have configurable RX internal delays.  This property is only
> +      used when the phy-mode or phy-connection-type is rgmii-id or rgmii-rxid.

Isn't this a property of the phy (this is the controller schema)? Looks 
like we have similar properties already and they go in phy nodes. Would 
be good to have a standard property, but let's be clear where it goes.

We need to add '-ps' as a standard unit suffix (in dt-schema) and then a 
type is not needed here.

Rob

^ permalink raw reply

* [PATCH v7] dt-bindings: spi: Convert DW SPI binding to DT schema
From: Serge Semin @ 2020-05-29 18:25 UTC (permalink / raw)
  To: Mark Brown, Rob Herring
  Cc: Serge Semin, Serge Semin, Rob Herring, Georgy Vlasov,
	Ramil Zaripov, Alexey Malahov, Thomas Bogendoerfer, Feng Tang,
	Andy Shevchenko, Arnd Bergmann, linux-mips, linux-spi, devicetree,
	linux-kernel

Modern device tree bindings are supposed to be created as YAML-files
in accordance with dt-schema. This commit replaces two DW SPI legacy
bare text bindings with YAML file. As before the bindings file states
that the corresponding dts node is supposed to be compatible either
with generic DW APB SSI controller or with Microsemi/Amazon/Renesas/Intel
vendors-specific controllers, to have registers, interrupts and clocks
properties. Though in case of Microsemi version of the controller
there must be two registers resources specified. Properties like
clock-names, reg-io-width, cs-gpio, num-cs, DMA and slave device
sub-nodes are optional.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Feng Tang <feng.tang@intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org

---

Changelog v7:
- Rebase on top of the spi/for-next branch.
- Add resets and reset-names properties, since Dinh Nguyen has slipped the
  patchset optional reset-support in right in front of my nose.
---
 .../bindings/spi/snps,dw-apb-ssi.txt          |  49 -------
 .../bindings/spi/snps,dw-apb-ssi.yaml         | 133 ++++++++++++++++++
 .../devicetree/bindings/spi/spi-dw.txt        |  24 ----
 3 files changed, 133 insertions(+), 73 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
 create mode 100644 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
 delete mode 100644 Documentation/devicetree/bindings/spi/spi-dw.txt

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
deleted file mode 100644
index 0f21407a7ea3..000000000000
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
+++ /dev/null
@@ -1,49 +0,0 @@
-Synopsys DesignWare AMBA 2.0 Synchronous Serial Interface.
-
-Required properties:
-- compatible : "snps,dw-apb-ssi" or "mscc,<soc>-spi", where soc is "ocelot" or
-  "jaguar2", or "amazon,alpine-dw-apb-ssi", or "snps,dwc-ssi-1.01a" or
-  "intel,keembay-ssi"
-- reg : The register base for the controller. For "mscc,<soc>-spi", a second
-  register set is required (named ICPU_CFG:SPI_MST)
-- interrupts : One interrupt, used by the controller.
-- #address-cells : <1>, as required by generic SPI binding.
-- #size-cells : <0>, also as required by generic SPI binding.
-- clocks : phandles for the clocks, see the description of clock-names below.
-   The phandle for the "ssi_clk" is required. The phandle for the "pclk" clock
-   is optional. If a single clock is specified but no clock-name, it is the
-   "ssi_clk" clock. If both clocks are listed, the "ssi_clk" must be first.
-
-Optional properties:
-- clock-names : Contains the names of the clocks:
-    "ssi_clk", for the core clock used to generate the external SPI clock.
-    "pclk", the interface clock, required for register access. If a clock domain
-     used to enable this clock then it should be named "pclk_clkdomain".
-- cs-gpios : Specifies the gpio pins to be used for chipselects.
-- num-cs : The number of chipselects. If omitted, this will default to 4.
-- reg-io-width : The I/O register width (in bytes) implemented by this
-  device.  Supported values are 2 or 4 (the default).
-- dmas : Phandle + identifiers of Tx and Rx DMA channels.
-- dma-names : Contains the names of the DMA channels. Must be "tx" and "rx".
-- resets : contains an entry for each entry in reset-names.
-	   See ../reset/reset.txt for details.
-- reset-names : must contain "spi"
-
-Child nodes as per the generic SPI binding.
-
-Example:
-
-	spi@fff00000 {
-		compatible = "snps,dw-apb-ssi";
-		reg = <0xfff00000 0x1000>;
-		interrupts = <0 154 4>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-		clocks = <&spi_m_clk>;
-		num-cs = <2>;
-		cs-gpios = <&gpio0 13 0>,
-			   <&gpio0 14 0>;
-		resets = <&rst SPIM0_RST>;
-		reset-names = "spi";
-	};
-
diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
new file mode 100644
index 000000000000..c62cbe79f00d
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -0,0 +1,133 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/snps,dw-apb-ssi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare AMBA 2.0 Synchronous Serial Interface
+
+maintainers:
+  - Mark Brown <broonie@kernel.org>
+
+allOf:
+  - $ref: "spi-controller.yaml#"
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - mscc,ocelot-spi
+              - mscc,jaguar2-spi
+    then:
+      properties:
+        reg:
+          minItems: 2
+
+properties:
+  compatible:
+    oneOf:
+      - description: Generic DW SPI Controller
+        enum:
+          - snps,dw-apb-ssi
+          - snps,dwc-ssi-1.01a
+      - description: Microsemi Ocelot/Jaguar2 SoC SPI Controller
+        items:
+          - enum:
+              - mscc,ocelot-spi
+              - mscc,jaguar2-spi
+          - const: snps,dw-apb-ssi
+      - description: Amazon Alpine SPI Controller
+        const: amazon,alpine-dw-apb-ssi
+      - description: Renesas RZ/N1 SPI Controller
+        items:
+          - const: renesas,rzn1-spi
+          - const: snps,dw-apb-ssi
+      - description: Intel Keem Bay SPI Controller
+        const: intel,keembay-ssi
+
+  reg:
+    minItems: 1
+    items:
+      - description: DW APB SSI controller memory mapped registers
+      - description: SPI MST region map
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+    items:
+      - description: SPI Controller reference clock source
+      - description: APB interface clock source
+
+  clock-names:
+    minItems: 1
+    items:
+      - const: ssi_clk
+      - const: pclk
+
+  resets:
+    maxItems: 1
+
+  reset-names:
+    const: spi
+
+  reg-io-width:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: I/O register width (in bytes) implemented by this device
+    default: 4
+    enum: [ 2, 4 ]
+
+  num-cs:
+    default: 4
+    minimum: 1
+    maximum: 4
+
+  dmas:
+    items:
+      - description: TX DMA Channel
+      - description: RX DMA Channel
+
+  dma-names:
+    items:
+      - const: tx
+      - const: rx
+
+patternProperties:
+  "^.*@[0-9a-f]+$":
+    type: object
+    properties:
+      reg:
+        minimum: 0
+        maximum: 3
+
+      spi-rx-bus-width:
+        const: 1
+
+      spi-tx-bus-width:
+        const: 1
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - interrupts
+  - clocks
+
+examples:
+  - |
+    spi@fff00000 {
+      compatible = "snps,dw-apb-ssi";
+      reg = <0xfff00000 0x1000>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+      interrupts = <0 154 4>;
+      clocks = <&spi_m_clk>;
+      num-cs = <2>;
+      cs-gpios = <&gpio0 13 0>,
+                 <&gpio0 14 0>;
+    };
+...
diff --git a/Documentation/devicetree/bindings/spi/spi-dw.txt b/Documentation/devicetree/bindings/spi/spi-dw.txt
deleted file mode 100644
index 7b63ed601990..000000000000
--- a/Documentation/devicetree/bindings/spi/spi-dw.txt
+++ /dev/null
@@ -1,24 +0,0 @@
-Synopsys DesignWare SPI master
-
-Required properties:
-- compatible: should be "snps,designware-spi"
-- #address-cells: see spi-bus.txt
-- #size-cells: see spi-bus.txt
-- reg: address and length of the spi master registers
-- interrupts: should contain one interrupt
-- clocks: spi clock phandle
-- num-cs: see spi-bus.txt
-
-Optional properties:
-- cs-gpios: see spi-bus.txt
-
-Example:
-
-spi: spi@4020a000 {
-	compatible = "snps,designware-spi";
-	interrupts = <11 1>;
-	reg = <0x4020a000 0x1000>;
-	clocks = <&pclk>;
-	num-cs = <2>;
-	cs-gpios = <&banka 0 0>;
-};
-- 
2.26.2


^ permalink raw reply related

* Re: [PATCH v4 0/2] soc: ti: add k3 platforms chipid module driver
From: Arnd Bergmann @ 2020-05-29 18:34 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: Santosh Shilimkar, Tero Kristo, Rob Herring, Lokesh Vutla, DTML,
	Dave Gerlach, Sekhar Nori, Linux ARM,
	linux-kernel@vger.kernel.org, Nishanth Menon
In-Reply-To: <a132765e-f4e4-a1e8-fb43-239188fecf1b@ti.com>

On Fri, May 29, 2020 at 8:22 PM Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
> On 12/05/2020 15:34, Grygorii Strashko wrote:

> >   .../bindings/soc/ti/k3-socinfo.yaml           |  40 +++++
> >   drivers/soc/ti/Kconfig                        |  10 ++
> >   drivers/soc/ti/Makefile                       |   1 +
> >   drivers/soc/ti/k3-socinfo.c                   | 152 ++++++++++++++++++
> >   4 files changed, 203 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/soc/ti/k3-socinfo.yaml
> >   create mode 100644 drivers/soc/ti/k3-socinfo.c
> >
>
> Any chances you can pick this up?

I merged a version of this driver from Santosh's pull request into the
arm/drviers tree yesterday.

Is there something missing?

       Arnd

^ permalink raw reply

* Re: [PATCH v3 3/6] hwmon: pmbus: adm1266: Add support for GPIOs
From: Guenter Roeck @ 2020-05-29 18:36 UTC (permalink / raw)
  To: alexandru.tachici, linux-hwmon, linux-kernel, devicetree; +Cc: robh+dt
In-Reply-To: <20200529130506.73511-4-alexandru.tachici@analog.com>

On 5/29/20 6:05 AM, alexandru.tachici@analog.com wrote:
> From: Alexandru Tachici <alexandru.tachici@analog.com>
> 
> Adm1266 exposes 9 GPIOs and 16 PDIOs which are currently read-only. They
> are controlled by the internal sequencing engine.
> 
> This patch makes adm1266 driver expose GPIOs and PDIOs to user-space
> using GPIO provider kernel api.
> 
> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
> ---
>  drivers/hwmon/pmbus/adm1266.c | 233 +++++++++++++++++++++++++++++++++-
>  1 file changed, 232 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index a7ef048a9a5c..190170300ef1 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
> @@ -6,7 +6,11 @@
>   * Copyright 2020 Analog Devices Inc.
>   */
>  
> +#include <linux/bitfield.h>
> +#include <linux/debugfs.h>
> +#include <linux/gpio/driver.h>
>  #include <linux/i2c.h>
> +#include <linux/i2c-smbus.h>
>  #include <linux/init.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> @@ -14,16 +18,243 @@
>  
>  #include "pmbus.h"
>  
> +#define ADM1266_PDIO_CONFIG	0xD4
> +#define ADM1266_GPIO_CONFIG	0xE1
> +#define ADM1266_PDIO_STATUS	0xE9
> +#define ADM1266_GPIO_STATUS	0xEA
> +
> +/* ADM1266 GPIO defines */
> +#define ADM1266_GPIO_NR			9
> +#define ADM1266_GPIO_FUNCTIONS(x)	FIELD_GET(BIT(0), x)
> +#define ADM1266_GPIO_INPUT_EN(x)	FIELD_GET(BIT(2), x)
> +#define ADM1266_GPIO_OUTPUT_EN(x)	FIELD_GET(BIT(3), x)
> +#define ADM1266_GPIO_OPEN_DRAIN(x)	FIELD_GET(BIT(4), x)
> +
> +/* ADM1266 PDIO defines */
> +#define ADM1266_PDIO_NR			16
> +#define ADM1266_PDIO_PIN_CFG(x)		FIELD_GET(GENMASK(15, 13), x)
> +#define ADM1266_PDIO_GLITCH_FILT(x)	FIELD_GET(GENMASK(12, 9), x)
> +#define ADM1266_PDIO_OUT_CFG(x)		FIELD_GET(GENMASK(2, 0), x)
> +
> +struct adm1266_data {
> +	struct pmbus_driver_info info;
> +	struct gpio_chip gc;
> +	const char *gpio_names[ADM1266_GPIO_NR + ADM1266_PDIO_NR];
> +	struct i2c_client *client;
> +};
> +
> +#if IS_ENABLED(CONFIG_GPIOLIB)

GPIOLIB is bool. IS_ENABLED suggests tristate, which won't happen. Just use #ifdef.

> +static const unsigned int adm1266_gpio_mapping[ADM1266_GPIO_NR][2] = {
> +	{1, 0},
> +	{2, 1},
> +	{3, 2},
> +	{4, 8},
> +	{5, 9},
> +	{6, 10},
> +	{7, 11},
> +	{8, 6},
> +	{9, 7},
> +};
> +
> +static const char *adm1266_names[ADM1266_GPIO_NR + ADM1266_PDIO_NR] = {
> +	"GPIO1", "GPIO2", "GPIO3", "GPIO4", "GPIO5", "GPIO6", "GPIO7", "GPIO8",
> +	"GPIO9", "PDIO1", "PDIO2", "PDIO3", "PDIO4", "PDIO5", "PDIO6",
> +	"PDIO7", "PDIO8", "PDIO9", "PDIO10", "PDIO11", "PDIO12", "PDIO13",
> +	"PDIO14", "PDIO15", "PDIO16",
> +};
> +
> +static int adm1266_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct adm1266_data *data = gpiochip_get_data(chip);
> +	u8 read_buf[PMBUS_BLOCK_MAX + 1];

Unnecessarily large. SMBUS_BLOCK_MAX is sufficient here.

> +	unsigned long pins_status;
> +	unsigned int pmbus_cmd;
> +	int ret;
> +
> +	if (offset < ADM1266_GPIO_NR)
> +		pmbus_cmd = ADM1266_GPIO_STATUS;
> +	else
> +		pmbus_cmd = ADM1266_PDIO_STATUS;
> +
> +	ret = i2c_smbus_read_block_data(data->client, pmbus_cmd,
> +					read_buf);
> +	if (ret < 0)
> +		return ret;
> +
> +	pins_status = read_buf[0] + (read_buf[1] << 8);
> +	if (offset < ADM1266_GPIO_NR)
> +		return test_bit(adm1266_gpio_mapping[offset][1], &pins_status);
> +	else

else after return is unnecessary.

> +		return test_bit(offset - ADM1266_GPIO_NR, &pins_status);
> +}
> +
> +static int adm1266_gpio_get_multiple(struct gpio_chip *chip,
> +				     unsigned long *mask,
> +				     unsigned long *bits)
> +{
> +	struct adm1266_data *data = gpiochip_get_data(chip);
> +	u8 gpio_data[PMBUS_BLOCK_MAX + 1];
> +	u8 pdio_data[PMBUS_BLOCK_MAX + 1];

This is quite some strain on the stack. I would suggest to use only
one buffer and handle gpio completely first, then pdio.

> +	unsigned long gpio_status;
> +	unsigned long pdio_status;
> +	unsigned int gpio_nr;
> +	int ret;
> +
> +	ret = i2c_smbus_read_block_data(data->client, ADM1266_GPIO_STATUS,
> +					gpio_data);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = i2c_smbus_read_block_data(data->client, ADM1266_PDIO_STATUS,
> +					pdio_data);
> +	if (ret < 0)
> +		return ret;
> +
> +	gpio_status = gpio_data[0] + (gpio_data[1] << 8);
> +	pdio_status = pdio_data[0] + (pdio_data[1] << 8);
> +	*bits = 0;
> +	for_each_set_bit(gpio_nr, mask, ADM1266_GPIO_NR) {
> +		if (test_bit(adm1266_gpio_mapping[gpio_nr][1], &gpio_status))
> +			set_bit(gpio_nr, bits);
> +	}
> +
> +	for_each_set_bit_from(gpio_nr, mask,
> +			      ADM1266_GPIO_NR + ADM1266_PDIO_STATUS) {
> +		if (test_bit(gpio_nr - ADM1266_GPIO_NR, &pdio_status))
> +			set_bit(gpio_nr, bits);
> +	}
> +
> +	return 0;
> +}
> +
> +static void adm1266_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> +{
> +	struct adm1266_data *data = gpiochip_get_data(chip);
> +	u8 write_buf[PMBUS_BLOCK_MAX + 1];

We know how much data we are going to write. Allocating more is a waste.
Given that it is only going to be one byte, you might as well just point
to it directly or use an u8 variable and point to that.

> +	u8 read_buf[PMBUS_BLOCK_MAX + 1];
> +	unsigned long gpio_config;
> +	unsigned long pdio_config;
> +	unsigned long pin_cfg;
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < ADM1266_GPIO_NR; i++) {
> +		write_buf[0] = adm1266_gpio_mapping[i][1];
> +		ret = pmbus_block_wr(data->client, ADM1266_GPIO_CONFIG, 1,
> +				     write_buf, read_buf);
> +		if (ret < 0)
> +			dev_err(&data->client->dev, "GPIOs scan failed(%d).\n",
> +				ret);
> +
So there was an error ...

> +		gpio_config = read_buf[0];

... and then happily report (random) data anyway ?

Besides, we are having logging noise again, this time on top of
the logging noise in pmbus_block_wr().

> +		seq_puts(s, adm1266_names[i]);
> +
> +		seq_puts(s, " ( ");
> +		if (!ADM1266_GPIO_FUNCTIONS(gpio_config)) {
> +			seq_puts(s, "high-Z )\n");
> +			continue;
> +		}
> +		if (ADM1266_GPIO_INPUT_EN(gpio_config))
> +			seq_puts(s, "input ");
> +		if (ADM1266_GPIO_OUTPUT_EN(gpio_config))
> +			seq_puts(s, "output ");
> +		if (ADM1266_GPIO_OPEN_DRAIN(gpio_config))
> +			seq_puts(s, "open-drain )\n");
> +		else
> +			seq_puts(s, "push-pull )\n");
> +	}
> +
> +	write_buf[0] = 0xFF;
> +	ret = pmbus_block_wr(data->client, ADM1266_PDIO_CONFIG, 1, write_buf,
> +			     read_buf);
> +	if (ret < 0)
> +		dev_err(&data->client->dev, "PDIOs scan failed(%d).\n", ret);
> +

Again: error followed by reporting (non-received) data anyway.
On top of that, 'ret' is not compared against ADM1266_PDIO_NR.
If less data is received than expected, the resulting output
will be random.

> +	for (i = 0; i < ADM1266_PDIO_NR; i++) {
> +		seq_puts(s, adm1266_names[ADM1266_GPIO_NR + i]);
> +
> +		pdio_config = read_buf[2 * i];
> +		pdio_config += (read_buf[2 * i + 1] << 8);
> +		pin_cfg = ADM1266_PDIO_PIN_CFG(pdio_config);
> +
> +		seq_puts(s, " ( ");
> +		if (!pin_cfg || pin_cfg > 5) {
> +			seq_puts(s, "high-Z )\n");
> +			continue;
> +		}
> +
> +		if (pin_cfg & BIT(0))
> +			seq_puts(s, "output ");
> +
> +		if (pin_cfg & BIT(1))
> +			seq_puts(s, "input ");
> +
> +		seq_puts(s, ")\n");
> +	}
> +}
> +
> +static int adm1266_config_gpio(struct adm1266_data *data)
> +{
> +	const char *name = dev_name(&data->client->dev);
> +	char *gpio_name;
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(data->gpio_names); i++) {
> +		gpio_name = devm_kasprintf(&data->client->dev, GFP_KERNEL,
> +					   "adm1266-%x-%s", data->client->addr,
> +					   adm1266_names[i]);
> +		if (!gpio_name)
> +			return -ENOMEM;
> +
> +		data->gpio_names[i] = gpio_name;
> +	}
> +
> +	data->gc.label = name;
> +	data->gc.parent = &data->client->dev;
> +	data->gc.owner = THIS_MODULE;
> +	data->gc.base = -1;
> +	data->gc.names = data->gpio_names;
> +	data->gc.ngpio = ARRAY_SIZE(data->gpio_names);
> +	data->gc.get = adm1266_gpio_get;
> +	data->gc.get_multiple = adm1266_gpio_get_multiple;
> +	data->gc.dbg_show = adm1266_gpio_dbg_show;
> +
> +	ret = devm_gpiochip_add_data(&data->client->dev, &data->gc, data);
> +	if (ret)
> +		dev_err(&data->client->dev, "GPIO registering failed (%d)\n",
> +			ret);
> +
> +	return ret;
> +}
> +#else
> +static inline int adm1266_config_gpio(struct adm1266_data *data)

inline is unnecessary. Let the compiler do its job.

> +{
> +	return 0;
> +}
> +#endif
> +
>  static int adm1266_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
>  	struct pmbus_driver_info *info;
> +	struct adm1266_data *data;
>  	u32 funcs;
> +	int ret;
>  	int i;
>  
> -	info = devm_kzalloc(&client->dev, sizeof(struct pmbus_driver_info),
> +	data = devm_kzalloc(&client->dev, sizeof(struct adm1266_data),
>  			    GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->client = client;
> +
> +	ret = adm1266_config_gpio(data);
> +	if (ret < 0)
> +		return ret;
>  
> +	info = &data->info;
>  	info->pages = 17;
>  	info->format[PSC_VOLTAGE_OUT] = linear;
>  	funcs = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
> 


^ permalink raw reply

* Re: [PATCH v5 02/11] dt-bindings: i2c: Discard i2c-slave flag from the DW I2C example
From: Andy Shevchenko @ 2020-05-29 18:42 UTC (permalink / raw)
  To: Serge Semin
  Cc: Rob Herring, Serge Semin, Jarkko Nikula, Wolfram Sang,
	Alexey Malahov, Thomas Bogendoerfer, Mika Westerberg, linux-mips,
	linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200529182256.3bp4uvvrvz3ddlrq@mobilestation>

On Fri, May 29, 2020 at 09:22:56PM +0300, Serge Semin wrote:
> On Fri, May 29, 2020 at 12:13:38PM -0600, Rob Herring wrote:
> > On Wed, May 27, 2020 at 06:33:51PM +0300, Serge Semin wrote:

> > you're sending 
> > new versions too fast. Give people time to review.
> 
> Yeah, you did. Sorry for sending the new versions very fast. Normally I prefer
> to keep up with comments so to past a particular maintainer review as fast as
> possible without long delays. In my experience the longer you wait, the lesser
> maintainers remember about your patchset, the harder for one to continue the
> next versions review.

Documentation/process/submitting-patches.rst:

"Wait for a minimum of one week before resubmitting or pinging reviewers -
possibly longer during busy times like merge windows."


-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v3 4/6] hwmon: pmbus: adm1266: add debugfs attr for states
From: Guenter Roeck @ 2020-05-29 18:43 UTC (permalink / raw)
  To: alexandru.tachici, linux-hwmon, linux-kernel, devicetree; +Cc: robh+dt
In-Reply-To: <20200529130506.73511-5-alexandru.tachici@analog.com>

On 5/29/20 6:05 AM, alexandru.tachici@analog.com wrote:
> From: Alexandru Tachici <alexandru.tachici@analog.com>
> 
> Add debugfs files for go_command and read_state.
> 
> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
> ---
>  drivers/hwmon/pmbus/adm1266.c | 47 +++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index 190170300ef1..85d6795b79d3 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
> @@ -19,6 +19,8 @@
>  #include "pmbus.h"
>  
>  #define ADM1266_PDIO_CONFIG	0xD4
> +#define ADM1266_GO_COMMAND	0xD8
> +#define ADM1266_READ_STATE	0xD9
>  #define ADM1266_GPIO_CONFIG	0xE1
>  #define ADM1266_PDIO_STATUS	0xE9
>  #define ADM1266_GPIO_STATUS	0xEA
> @@ -41,6 +43,7 @@ struct adm1266_data {
>  	struct gpio_chip gc;
>  	const char *gpio_names[ADM1266_GPIO_NR + ADM1266_PDIO_NR];
>  	struct i2c_client *client;
> +	struct dentry *debugfs_dir;
>  };
>  
>  #if IS_ENABLED(CONFIG_GPIOLIB)
> @@ -234,6 +237,48 @@ static inline int adm1266_config_gpio(struct adm1266_data *data)
>  }
>  #endif
>  
> +static int adm1266_get_state_op(void *pdata, u64 *state)
> +{
> +	struct adm1266_data *data = pdata;
> +	int ret;
> +
> +	ret = i2c_smbus_read_word_data(data->client, ADM1266_READ_STATE);
> +	if (ret < 0)
> +		return ret;
> +
> +	*state = ret;
> +
> +	return 0;
> +}
> +
> +static int adm1266_set_go_command_op(void *pdata, u64 val)
> +{
> +	struct adm1266_data *data = pdata;
> +	u8 reg;
> +
> +	reg = FIELD_GET(GENMASK(4, 0), val);
> +
> +	return i2c_smbus_write_word_data(data->client, ADM1266_GO_COMMAND, reg);
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(go_command_fops, NULL, adm1266_set_go_command_op,
> +			 "%llu\n");
> +DEFINE_DEBUGFS_ATTRIBUTE(read_state_fops, adm1266_get_state_op, NULL, "%llu\n");
> +
> +static void adm1266_debug_init(struct adm1266_data *data)
> +{
> +	struct dentry *root;
> +	char dir_name[30];
> +
> +	sprintf(dir_name, "adm1266-%x_debugfs", data->client->addr);
> +	root = debugfs_create_dir(dir_name, NULL);
> +	data->debugfs_dir = root;
> +	debugfs_create_file_unsafe("go_command", 0200, root, data,
> +				   &go_command_fops);

I am not entirely sure what this does, but from the description in the datasheet
it is way too critical to support as debugfs command. Anyone believing this
is needed should use ioctl commands instead.


> +	debugfs_create_file_unsafe("read_state", 0400, root, data,
> +				   &read_state_fops);
> +}
> +

We have standard pmbus debug functions. Please use it.

>  static int adm1266_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
> @@ -254,6 +299,8 @@ static int adm1266_probe(struct i2c_client *client,
>  	if (ret < 0)
>  		return ret;
>  
> +	adm1266_debug_init(data);
> +
>  	info = &data->info;
>  	info->pages = 17;
>  	info->format[PSC_VOLTAGE_OUT] = linear;
> 


^ permalink raw reply

* Re: [PATCH v3 5/6] hwmon: pmbus: adm1266: read blackbox
From: Guenter Roeck @ 2020-05-29 18:45 UTC (permalink / raw)
  To: alexandru.tachici, linux-hwmon, linux-kernel, devicetree; +Cc: robh+dt
In-Reply-To: <20200529130506.73511-6-alexandru.tachici@analog.com>

On 5/29/20 6:05 AM, alexandru.tachici@analog.com wrote:
> From: Alexandru Tachici <alexandru.tachici@analog.com>
> 
> Use the nvmem kernel api to expose the black box
> chip functionality to userspace.
> 

This needs to be split into two functions: Add nvmem support, add
debugfs file.

Guenter

> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
> ---
>  drivers/hwmon/pmbus/adm1266.c | 160 ++++++++++++++++++++++++++++++++++
>  1 file changed, 160 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index 85d6795b79d3..831156004087 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
> @@ -14,14 +14,19 @@
>  #include <linux/init.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/nvmem-provider.h>
>  #include <linux/slab.h>
>  
>  #include "pmbus.h"
>  
> +#define ADM1266_BLACKBOX_CONFIG	0xD3
>  #define ADM1266_PDIO_CONFIG	0xD4
>  #define ADM1266_GO_COMMAND	0xD8
>  #define ADM1266_READ_STATE	0xD9
> +#define ADM1266_READ_BLACKBOX	0xDE
>  #define ADM1266_GPIO_CONFIG	0xE1
> +#define ADM1266_BLACKBOX_INFO	0xE6
>  #define ADM1266_PDIO_STATUS	0xE9
>  #define ADM1266_GPIO_STATUS	0xEA
>  
> @@ -38,12 +43,26 @@
>  #define ADM1266_PDIO_GLITCH_FILT(x)	FIELD_GET(GENMASK(12, 9), x)
>  #define ADM1266_PDIO_OUT_CFG(x)		FIELD_GET(GENMASK(2, 0), x)
>  
> +#define ADM1266_BLACKBOX_OFFSET		0x7F700
> +#define ADM1266_BLACKBOX_SIZE		64
> +
>  struct adm1266_data {
>  	struct pmbus_driver_info info;
>  	struct gpio_chip gc;
>  	const char *gpio_names[ADM1266_GPIO_NR + ADM1266_PDIO_NR];
>  	struct i2c_client *client;
>  	struct dentry *debugfs_dir;
> +	struct nvmem_config nvmem_config;
> +	struct nvmem_device *nvmem;
> +	u8 *dev_mem;
> +};
> +
> +static const struct nvmem_cell_info adm1266_nvmem_cells[] = {
> +	{
> +		.name           = "blackbox",
> +		.offset         = ADM1266_BLACKBOX_OFFSET,
> +		.bytes          = 2048,
> +	},
>  };
>  
>  #if IS_ENABLED(CONFIG_GPIOLIB)
> @@ -261,6 +280,28 @@ static int adm1266_set_go_command_op(void *pdata, u64 val)
>  	return i2c_smbus_write_word_data(data->client, ADM1266_GO_COMMAND, reg);
>  }
>  
> +static int adm1266_blackbox_information_read(struct seq_file *s, void *pdata)
> +{
> +	struct device *dev = s->private;
> +	struct i2c_client *client = to_i2c_client(dev);
> +	u8 read_buf[PMBUS_BLOCK_MAX + 1];
> +	unsigned int latest_id;
> +	int ret;
> +
> +	ret = i2c_smbus_read_block_data(client, ADM1266_BLACKBOX_INFO,
> +					read_buf);
> +	if (ret < 0)
> +		return ret;
> +
> +	seq_puts(s, "BLACKBOX_INFORMATION:\n");
> +	latest_id = read_buf[0] + (read_buf[1] << 8);
> +	seq_printf(s, "Black box ID: %x\n", latest_id);
> +	seq_printf(s, "Logic index: %x\n", read_buf[2]);
> +	seq_printf(s, "Record count: %x\n", read_buf[3]);
> +
> +	return 0;
> +}
> +
>  DEFINE_DEBUGFS_ATTRIBUTE(go_command_fops, NULL, adm1266_set_go_command_op,
>  			 "%llu\n");
>  DEFINE_DEBUGFS_ATTRIBUTE(read_state_fops, adm1266_get_state_op, NULL, "%llu\n");
> @@ -277,6 +318,121 @@ static void adm1266_debug_init(struct adm1266_data *data)
>  				   &go_command_fops);
>  	debugfs_create_file_unsafe("read_state", 0400, root, data,
>  				   &read_state_fops);
> +	debugfs_create_devm_seqfile(&data->client->dev, "blackbox_information",
> +				    root, adm1266_blackbox_information_read);
> +}
> +
> +static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *buf)
> +{
> +	u8 write_buf[PMBUS_BLOCK_MAX + 1];
> +	u8 read_buf[PMBUS_BLOCK_MAX + 1];
> +	int record_count;
> +	int ret;
> +	int i;
> +
> +	ret = i2c_smbus_read_block_data(data->client, ADM1266_BLACKBOX_INFO,
> +					read_buf);
> +	if (ret < 0)
> +		return ret;
> +
> +	record_count = read_buf[3];
> +
> +	for (i = 0; i < record_count; i++) {
> +		write_buf[0] = i;
> +		ret = pmbus_block_wr(data->client, ADM1266_READ_BLACKBOX, 1,
> +				     write_buf, buf);
> +		if (ret < 0)
> +			return ret;
> +
> +		buf += ADM1266_BLACKBOX_SIZE;
> +	}
> +
> +	return 0;
> +}
> +
> +static bool adm1266_cell_is_accessed(const struct nvmem_cell_info *mem_cell,
> +				     unsigned int offset, size_t bytes)
> +{
> +	unsigned int start_addr = offset;
> +	unsigned int end_addr = offset + bytes;
> +	unsigned int cell_start = mem_cell->offset;
> +	unsigned int cell_end = mem_cell->offset + mem_cell->bytes;
> +
> +	if (start_addr <= cell_end && cell_start <= end_addr)
> +		return true;
> +
> +	return false;
> +}
> +
> +static int adm1266_read_mem_cell(struct adm1266_data *data,
> +				 const struct nvmem_cell_info *mem_cell)
> +{
> +	u8 *mem_offset;
> +	int ret;
> +
> +	switch (mem_cell->offset) {
> +	case ADM1266_BLACKBOX_OFFSET:
> +		mem_offset = data->dev_mem + mem_cell->offset;
> +		ret = adm1266_nvmem_read_blackbox(data, mem_offset);
> +		if (ret)
> +			dev_err(&data->client->dev, "Could not read blackbox!");
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int adm1266_nvmem_read(void *priv, unsigned int offset, void *val,
> +			      size_t bytes)
> +{
> +	const struct nvmem_cell_info *mem_cell;
> +	struct adm1266_data *data = priv;
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < data->nvmem_config.ncells; i++) {
> +		mem_cell = &adm1266_nvmem_cells[i];
> +		if (!adm1266_cell_is_accessed(mem_cell, offset, bytes))
> +			continue;
> +
> +		ret = adm1266_read_mem_cell(data, mem_cell);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	memcpy(val, data->dev_mem + offset, bytes);
> +
> +	return 0;
> +}
> +
> +static int adm1266_config_nvmem(struct adm1266_data *data)
> +{
> +	data->nvmem_config.name = dev_name(&data->client->dev);
> +	data->nvmem_config.dev = &data->client->dev;
> +	data->nvmem_config.root_only = true;
> +	data->nvmem_config.read_only = true;
> +	data->nvmem_config.owner = THIS_MODULE;
> +	data->nvmem_config.reg_read = adm1266_nvmem_read;
> +	data->nvmem_config.cells = adm1266_nvmem_cells;
> +	data->nvmem_config.ncells = ARRAY_SIZE(adm1266_nvmem_cells);
> +	data->nvmem_config.priv = data;
> +	data->nvmem_config.stride = 1;
> +	data->nvmem_config.word_size = 1;
> +	data->nvmem_config.size = 0x80000;
> +
> +	data->nvmem = nvmem_register(&data->nvmem_config);
> +	if (IS_ERR(data->nvmem)) {
> +		dev_err(&data->client->dev, "Could not register nvmem!");
> +		return PTR_ERR(data->nvmem);
> +	}
> +
> +	data->dev_mem = devm_kzalloc(&data->client->dev,
> +				     data->nvmem_config.size,
> +				     GFP_KERNEL);
> +	if (!data->dev_mem)
> +		return -ENOMEM;
> +
> +	return 0;
>  }
>  
>  static int adm1266_probe(struct i2c_client *client,
> @@ -299,6 +455,10 @@ static int adm1266_probe(struct i2c_client *client,
>  	if (ret < 0)
>  		return ret;
>  
> +	ret = adm1266_config_nvmem(data);
> +	if (ret < 0)
> +		return ret;
> +
>  	adm1266_debug_init(data);
>  
>  	info = &data->info;
> 


^ permalink raw reply

* Re: [PATCH v5 02/11] dt-bindings: i2c: Discard i2c-slave flag from the DW I2C example
From: Serge Semin @ 2020-05-29 18:45 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, Rob Herring, Jarkko Nikula, Wolfram Sang,
	Alexey Malahov, Thomas Bogendoerfer, Mika Westerberg, linux-mips,
	linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200529184201.GX1634618@smile.fi.intel.com>

On Fri, May 29, 2020 at 09:42:01PM +0300, Andy Shevchenko wrote:
> On Fri, May 29, 2020 at 09:22:56PM +0300, Serge Semin wrote:
> > On Fri, May 29, 2020 at 12:13:38PM -0600, Rob Herring wrote:
> > > On Wed, May 27, 2020 at 06:33:51PM +0300, Serge Semin wrote:
> 
> > > you're sending 
> > > new versions too fast. Give people time to review.
> > 
> > Yeah, you did. Sorry for sending the new versions very fast. Normally I prefer
> > to keep up with comments so to past a particular maintainer review as fast as
> > possible without long delays. In my experience the longer you wait, the lesser
> > maintainers remember about your patchset, the harder for one to continue the
> > next versions review.
> 

> Documentation/process/submitting-patches.rst:
> 
> "Wait for a minimum of one week before resubmitting or pinging reviewers -
> possibly longer during busy times like merge windows."

Good to know what I already know.) How much do you personally wait before
resubmitting? In my experience reviewing your DW APB GPIO patches, no longer
than a few hours.

-Sergey

> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

^ permalink raw reply

* Re: [PATCH v5 02/11] dt-bindings: i2c: Discard i2c-slave flag from the DW I2C example
From: Serge Semin @ 2020-05-29 18:58 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, Rob Herring, Jarkko Nikula, Wolfram Sang,
	Alexey Malahov, Thomas Bogendoerfer, Mika Westerberg, linux-mips,
	linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200529184534.wyyv5i7hcto5y3d3@mobilestation>

On Fri, May 29, 2020 at 09:45:37PM +0300, Serge Semin wrote:
> On Fri, May 29, 2020 at 09:42:01PM +0300, Andy Shevchenko wrote:
> > On Fri, May 29, 2020 at 09:22:56PM +0300, Serge Semin wrote:
> > > On Fri, May 29, 2020 at 12:13:38PM -0600, Rob Herring wrote:
> > > > On Wed, May 27, 2020 at 06:33:51PM +0300, Serge Semin wrote:
> > 
> > > > you're sending 
> > > > new versions too fast. Give people time to review.
> > > 
> > > Yeah, you did. Sorry for sending the new versions very fast. Normally I prefer
> > > to keep up with comments so to past a particular maintainer review as fast as
> > > possible without long delays. In my experience the longer you wait, the lesser
> > > maintainers remember about your patchset, the harder for one to continue the
> > > next versions review.
> > 
> 

> > Documentation/process/submitting-patches.rst:
> > 
> > "Wait for a minimum of one week before resubmitting or pinging reviewers -
> > possibly longer during busy times like merge windows."
> 
> Good to know what I already know.) How much do you personally wait before
> resubmitting? In my experience reviewing your DW APB GPIO patches, no longer
> than a few hours.

Moreover the statement you cited is about the patches, which doesn't get any
attention from the maintainers/reviewers for quite some time. In this case I
normally resubmit the patches no sooner than a week. I was talking about the
situation when you get the review comments, which need to be addressed.

-Sergey

> 
> -Sergey
> 
> > 
> > 
> > -- 
> > With Best Regards,
> > Andy Shevchenko
> > 
> > 

^ permalink raw reply

* Re: [PATCH net-next v4 3/4] dt-bindings: net: Add RGMII internal delay for DP83869
From: Rob Herring @ 2020-05-29 19:03 UTC (permalink / raw)
  To: Dan Murphy
  Cc: andrew, f.fainelli, hkallweit1, davem, netdev, linux-kernel,
	devicetree
In-Reply-To: <20200527164934.28651-4-dmurphy@ti.com>

On Wed, May 27, 2020 at 11:49:33AM -0500, Dan Murphy wrote:
> Add the internal delay values into the header and update the binding
> with the internal delay properties.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  .../devicetree/bindings/net/ti,dp83869.yaml      | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ti,dp83869.yaml b/Documentation/devicetree/bindings/net/ti,dp83869.yaml
> index 5b69ef03bbf7..2971dd3fc039 100644
> --- a/Documentation/devicetree/bindings/net/ti,dp83869.yaml
> +++ b/Documentation/devicetree/bindings/net/ti,dp83869.yaml
> @@ -64,6 +64,20 @@ properties:
>         Operational mode for the PHY.  If this is not set then the operational
>         mode is set by the straps. see dt-bindings/net/ti-dp83869.h for values
>  
> +  rx-internal-delay-ps:
> +    $ref: "#/properties/rx-internal-delay-ps"

This just creates a circular reference which probably blows up.

> +    description: Delay is in pico seconds
> +    enum: [ 250, 500, 750, 1000, 1250, 1500, 1750, 2000, 2250, 2500, 2750, 3000,
> +            3250, 3500, 3750, 4000 ]
> +    default: 2000
> +
> +  tx-internal-delay-ps:
> +    $ref: "#/properties/tx-internal-delay-ps"
> +    description: Delay is in pico seconds
> +    enum: [ 250, 500, 750, 1000, 1250, 1500, 1750, 2000, 2250, 2500, 2750, 3000,
> +            3250, 3500, 3750, 4000 ]
> +    default: 2000
> +
>  required:
>    - reg
>  
> @@ -80,5 +94,7 @@ examples:
>          ti,op-mode = <DP83869_RGMII_COPPER_ETHERNET>;
>          ti,max-output-impedance = "true";
>          ti,clk-output-sel = <DP83869_CLK_O_SEL_CHN_A_RCLK>;
> +        rx-internal-delay-ps = <2000>;
> +        tx-internal-delay-ps = <2000>;
>        };
>      };
> -- 
> 2.26.2
> 

^ permalink raw reply

* Re: [PATCH v2 11/12] dt-bindings: iio: imu: Add inv_icm42600 documentation
From: Rob Herring @ 2020-05-29 19:05 UTC (permalink / raw)
  To: Jean-Baptiste Maneyrol
  Cc: jic23, mchehab+huawei, davem, gregkh, linux-iio, devicetree,
	linux-kernel
In-Reply-To: <20200527185711.21331-12-jmaneyrol@invensense.com>

On Wed, May 27, 2020 at 08:57:10PM +0200, Jean-Baptiste Maneyrol wrote:
> Document the ICM-426xxx devices devicetree bindings.
> 
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> ---
>  .../bindings/iio/imu/invensense,icm42600.yaml | 86 +++++++++++++++++++
>  1 file changed, 86 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> new file mode 100644
> index 000000000000..c5b046e0ce36
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/imu/invensense,icm42600.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: InvenSense ICM-426xx Inertial Measurement Unit
> +
> +maintainers:
> +  - Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> +
> +description: |
> +  6-axis MotionTracking device that combines a 3-axis gyroscope and a 3-axis accelerometer.
> +
> +  It has a configurable host interface that supports I3C, I2C and SPI serial communication, features a 2kB FIFO and
> +  2 programmable interrupts with ultra-low-power wake-on-motion support to minimize system power consumption.
> +
> +  Other industry-leading features include InvenSense on-chip APEX Motion Processing engine for gesture recognition,
> +  activity classification, and pedometer, along with programmable digital filters, and an embedded temperature sensor.

Wrap lines at <80 chars.

> +
> +  https://invensense.tdk.com/wp-content/uploads/2020/03/DS-000292-ICM-42605-v1.4.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - invensense,icm42600
> +      - invensense,icm42602
> +      - invensense,icm42605
> +      - invensense,icm42622
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  drive-open-drain:
> +    type: boolean
> +
> +  vdd-supply:
> +    description: Regulator that provides power to the sensor
> +
> +  vddio-supply:
> +    description: Regulator that provides power to the bus
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        icm42605@68 {
> +          compatible = "invensense,icm42605";
> +          reg = <0x68>;
> +          interrupt-parent = <&gpio2>;
> +          interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> +          vdd-supply = <&vdd>;
> +          vddio-supply = <&vddio>;
> +        };
> +    };
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    spi0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        icm42602@0 {
> +          compatible = "invensense,icm42602";
> +          reg = <0>;
> +          spi-max-frequency = <24000000>;
> +          spi-cpha;
> +          spi-cpol;
> +          interrupt-parent = <&gpio1>;
> +          interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
> +          vdd-supply = <&vdd>;
> +          vddio-supply = <&vddio>;
> +        };
> +    };
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: [PATCH v14 1/3] dt-bindings: i2c: npcm7xx: add NPCM I2C controller
From: Rob Herring @ 2020-05-29 19:07 UTC (permalink / raw)
  To: Tali Perry
  Cc: venture, yuenn, andriy.shevchenko, linux-arm-kernel, ofery,
	openbmc, brendanhiggins, benjaminfair, linux-kernel, linux-i2c,
	kfting, devicetree, robh+dt, avifishman70, wsa, tmaimon77
In-Reply-To: <20200527200820.47359-2-tali.perry1@gmail.com>

On Wed, 27 May 2020 23:08:18 +0300, Tali Perry wrote:
> Added device tree binding documentation for Nuvoton BMC
> NPCM I2C controller.
> 
> Signed-off-by: Tali Perry <tali.perry1@gmail.com>
> ---
>  .../bindings/i2c/nuvoton,npcm7xx-i2c.yaml     | 62 +++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH 1/4] dt-bindings: thermal: rcar-thermal: Add device tree support for r8a7742
From: Rob Herring @ 2020-05-29 19:09 UTC (permalink / raw)
  To: Lad Prabhakar
  Cc: linux-kernel, Magnus Damm, Geert Uytterhoeven, Rob Herring,
	Prabhakar, linux-renesas-soc, devicetree
In-Reply-To: <1590614320-30160-2-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com>

On Wed, 27 May 2020 22:18:37 +0100, Lad Prabhakar wrote:
> Add thermal sensor support for r8a7742 SoC. The Renesas RZ/G1H
> (r8a7742) thermal sensor module is identical to the R-Car Gen2 family.
> 
> No driver change is needed due to the fallback compatible value
> "renesas,rcar-gen2-thermal".
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> ---
>  Documentation/devicetree/bindings/thermal/rcar-thermal.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Applied, thanks!

^ permalink raw reply

* Re: [PATCH 3/4] dt-bindings: timer: renesas,cmt: Document r8a7742 CMT support
From: Rob Herring @ 2020-05-29 19:14 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Lad Prabhakar, Magnus Damm, Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux Kernel Mailing List, Prabhakar
In-Reply-To: <CAMuHMdX+m7WixJiNJyrpHxjnp5vFHi3ULuT7QgPOA93NE1XiHQ@mail.gmail.com>

On Fri, May 29, 2020 at 02:53:02PM +0200, Geert Uytterhoeven wrote:
> Hi Prabhakar,
> 
> On Wed, May 27, 2020 at 11:19 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Document SoC specific compatible strings for r8a7742. No driver change
> > is needed as the fallback strings will activate the right code.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Please note this DT binding is under yamlization, cfr.
> "[PATCH v2] dt-bindings: timer: renesas: cmt: Convert to json-schema"
> (20200505155127.4836-1-geert+renesas@glider.be).

Do I need to pick that one up? Doesn't look like it's been applied 
AFAICT.

Rob

^ permalink raw reply

* Re: [PATCH v13 3/7] dt-bindings: clock: Add documentation for X1830 bindings.
From: Rob Herring @ 2020-05-29 19:14 UTC (permalink / raw)
  To: 周琰杰 (Zhou Yanjie)
  Cc: mturquette, robh+dt, sboyd, aric.pzqi, rick.tyliu, yanfei.li,
	zhenwenjin, sernia.zhou, paul, dongsheng.qiu, devicetree,
	linux-clk, linux-kernel
In-Reply-To: <20200528031549.13846-4-zhouyanjie@wanyeetech.com>

On Thu, 28 May 2020 11:15:45 +0800, 周琰杰 (Zhou Yanjie) wrote:
> Add documentation for the clock bindings of the X1830 Soc from Ingenic.
> 
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> 
> Notes:
>     v11:
>     New patch, split from [3/6] in v10.
> 
>     v11->v12:
>     No change.
> 
>     v12->v13:
>     No change.
> 
>  Documentation/devicetree/bindings/clock/ingenic,cgu.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [PATCH v2 0/4] pinctrl: bcm2835: Support for wake-up interrupts
From: Florian Fainelli @ 2020-05-29 19:15 UTC (permalink / raw)
  To: linux-kernel
  Cc: Florian Fainelli, Linus Walleij, Rob Herring, Ray Jui,
	Scott Branden,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	Nicolas Saenz Julienne, Stefan Wahren, Geert Uytterhoeven,
	Matti Vaittinen, open list:PIN CONTROL SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE

Hi Linus,

This patch series updates the bcm2835 pinctrl driver to support
the BCM7211 SoC which is quite similar to 2711 (Raspberry Pi 4)
except that it also supports wake-up interrupts.

Thanks!

Changes in v2:

- fixed patch #3 to reference the correct data structure (Stefan)
- fixed patch #4 to use conditional initialization and fetching of
  interrupt resources to limit the memory overhead for non-7211 chips

Florian Fainelli (4):
  dt-bindings: pinctrl: Document 7211 compatible for
    brcm,bcm2835-gpio.txt
  dt-bindings: pinctrl: Document optional BCM7211 wake-up interrupts
  pinctrl: bcm2835: Match BCM7211 compatible string
  pinctrl: bcm2835: Add support for wake-up interrupts

 .../bindings/pinctrl/brcm,bcm2835-gpio.txt    |  5 +-
 drivers/pinctrl/bcm/pinctrl-bcm2835.c         | 80 ++++++++++++++++++-
 2 files changed, 83 insertions(+), 2 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH v2 3/4] pinctrl: bcm2835: Match BCM7211 compatible string
From: Florian Fainelli @ 2020-05-29 19:15 UTC (permalink / raw)
  To: linux-kernel
  Cc: Florian Fainelli, Linus Walleij, Rob Herring, Ray Jui,
	Scott Branden,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	Nicolas Saenz Julienne, Stefan Wahren, Geert Uytterhoeven,
	Matti Vaittinen, open list:PIN CONTROL SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
In-Reply-To: <20200529191522.27938-1-f.fainelli@gmail.com>

The BCM7211 SoC uses the same pinconf_ops as the ones defined for the
BCM2711 SoC, match the compatible string and use the correct set of
options.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 06bd2b70af3c..1b00d93aa66e 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -1137,6 +1137,10 @@ static const struct of_device_id bcm2835_pinctrl_match[] = {
 		.compatible = "brcm,bcm2711-gpio",
 		.data = &bcm2711_plat_data,
 	},
+	{
+		.compatible = "brcm,bcm7211-gpio",
+		.data = &bcm2711_plat_data,
+	},
 	{}
 };
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 4/4] pinctrl: bcm2835: Add support for wake-up interrupts
From: Florian Fainelli @ 2020-05-29 19:15 UTC (permalink / raw)
  To: linux-kernel
  Cc: Florian Fainelli, Linus Walleij, Rob Herring, Ray Jui,
	Scott Branden,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	Nicolas Saenz Julienne, Stefan Wahren, Geert Uytterhoeven,
	Matti Vaittinen, open list:PIN CONTROL SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
In-Reply-To: <20200529191522.27938-1-f.fainelli@gmail.com>

Leverage the IRQCHIP_MASK_ON_SUSPEND flag in order to avoid having to
specifically treat the GPIO interrupts during suspend and resume, and
simply implement an irq_set_wake() callback that is responsible for
enabling the parent wake-up interrupt as a wake-up interrupt.

To avoid allocating unnecessary resources for other chips, the wake-up
interrupts are only initialized if we have a brcm,bcm7211-gpio
compatibility string.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 76 ++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index 1b00d93aa66e..1fbf067a3eed 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -19,6 +19,7 @@
 #include <linux/irq.h>
 #include <linux/irqdesc.h>
 #include <linux/init.h>
+#include <linux/interrupt.h>
 #include <linux/of_address.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
@@ -76,6 +77,7 @@
 struct bcm2835_pinctrl {
 	struct device *dev;
 	void __iomem *base;
+	int *wake_irq;
 
 	/* note: locking assumes each bank will have its own unsigned long */
 	unsigned long enabled_irq_map[BCM2835_NUM_BANKS];
@@ -435,6 +437,11 @@ static void bcm2835_gpio_irq_handler(struct irq_desc *desc)
 	chained_irq_exit(host_chip, desc);
 }
 
+static irqreturn_t bcm2835_gpio_wake_irq_handler(int irq, void *dev_id)
+{
+	return IRQ_HANDLED;
+}
+
 static inline void __bcm2835_gpio_irq_config(struct bcm2835_pinctrl *pc,
 	unsigned reg, unsigned offset, bool enable)
 {
@@ -634,6 +641,34 @@ static void bcm2835_gpio_irq_ack(struct irq_data *data)
 	bcm2835_gpio_set_bit(pc, GPEDS0, gpio);
 }
 
+static int bcm2835_gpio_irq_set_wake(struct irq_data *data, unsigned int on)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct bcm2835_pinctrl *pc = gpiochip_get_data(chip);
+	unsigned gpio = irqd_to_hwirq(data);
+	unsigned int irqgroup;
+	int ret = -EINVAL;
+
+	if (!pc->wake_irq)
+		return ret;
+
+	if (gpio <= 27)
+		irqgroup = 0;
+	else if (gpio >= 28 && gpio <= 45)
+		irqgroup = 1;
+	else if (gpio >= 46 && gpio <= 53)
+		irqgroup = 2;
+	else
+		return ret;
+
+	if (on)
+		ret = enable_irq_wake(pc->wake_irq[irqgroup]);
+	else
+		ret = disable_irq_wake(pc->wake_irq[irqgroup]);
+
+	return ret;
+}
+
 static struct irq_chip bcm2835_gpio_irq_chip = {
 	.name = MODULE_NAME,
 	.irq_enable = bcm2835_gpio_irq_enable,
@@ -642,6 +677,8 @@ static struct irq_chip bcm2835_gpio_irq_chip = {
 	.irq_ack = bcm2835_gpio_irq_ack,
 	.irq_mask = bcm2835_gpio_irq_disable,
 	.irq_unmask = bcm2835_gpio_irq_enable,
+	.irq_set_wake = bcm2835_gpio_irq_set_wake,
+	.flags = IRQCHIP_MASK_ON_SUSPEND,
 };
 
 static int bcm2835_pctl_get_groups_count(struct pinctrl_dev *pctldev)
@@ -1154,6 +1191,7 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 	struct resource iomem;
 	int err, i;
 	const struct of_device_id *match;
+	int is_7211 = 0;
 
 	BUILD_BUG_ON(ARRAY_SIZE(bcm2835_gpio_pins) != BCM2711_NUM_GPIOS);
 	BUILD_BUG_ON(ARRAY_SIZE(bcm2835_gpio_groups) != BCM2711_NUM_GPIOS);
@@ -1180,6 +1218,7 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 		return -EINVAL;
 
 	pdata = match->data;
+	is_7211 = of_device_is_compatible(np, "brcm,bcm7211-gpio");
 
 	pc->gpio_chip = *pdata->gpio_chip;
 	pc->gpio_chip.parent = dev;
@@ -1214,6 +1253,15 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 				     GFP_KERNEL);
 	if (!girq->parents)
 		return -ENOMEM;
+
+	if (is_7211) {
+		pc->wake_irq = devm_kcalloc(dev, BCM2835_NUM_IRQS,
+					    sizeof(*pc->wake_irq),
+					    GFP_KERNEL);
+		if (!pc->wake_irq)
+			return -ENOMEM;
+	}
+
 	/*
 	 * Use the same handler for all groups: this is necessary
 	 * since we use one gpiochip to cover all lines - the
@@ -1221,8 +1269,34 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 	 * bank that was firing the IRQ and look up the per-group
 	 * and bank data.
 	 */
-	for (i = 0; i < BCM2835_NUM_IRQS; i++)
+	for (i = 0; i < BCM2835_NUM_IRQS; i++) {
+		int len;
+		char *name;
+
 		girq->parents[i] = irq_of_parse_and_map(np, i);
+		if (!is_7211)
+			continue;
+
+		/* Skip over the all banks interrupts */
+		pc->wake_irq[i] = irq_of_parse_and_map(np, i +
+						       BCM2835_NUM_IRQS + 1);
+
+		len = strlen(dev_name(pc->dev)) + 16;
+		name = devm_kzalloc(pc->dev, len, GFP_KERNEL);
+		if (!name)
+			return -ENOMEM;
+
+		snprintf(name, len, "%s:bank%d", dev_name(pc->dev), i);
+
+		/* These are optional interrupts */
+		err = devm_request_irq(dev, pc->wake_irq[i],
+				       bcm2835_gpio_wake_irq_handler,
+				       IRQF_SHARED, name, pc);
+		if (err)
+			dev_warn(dev, "unable to request wake IRQ %d\n",
+				 pc->wake_irq[i]);
+	}
+
 	girq->default_type = IRQ_TYPE_NONE;
 	girq->handler = handle_level_irq;
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 2/4] dt-bindings: pinctrl: Document optional BCM7211 wake-up interrupts
From: Florian Fainelli @ 2020-05-29 19:15 UTC (permalink / raw)
  To: linux-kernel
  Cc: Florian Fainelli, Linus Walleij, Rob Herring, Ray Jui,
	Scott Branden,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	Nicolas Saenz Julienne, Stefan Wahren, Geert Uytterhoeven,
	Matti Vaittinen, open list:PIN CONTROL SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
In-Reply-To: <20200529191522.27938-1-f.fainelli@gmail.com>

BCM7211 supports wake-up interrupts in the form of optional interrupt
lines, one per bank, plus the "all banks" interrupt line.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 .../devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt         | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
index dfc67b90591c..5682b2010e50 100644
--- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
@@ -16,7 +16,9 @@ Required properties:
   second cell is used to specify optional parameters:
   - bit 0 specifies polarity (0 for normal, 1 for inverted)
 - interrupts : The interrupt outputs from the controller. One interrupt per
-  individual bank followed by the "all banks" interrupt.
+  individual bank followed by the "all banks" interrupt. For BCM7211, an
+  additional set of per-bank interrupt line and an "all banks" wake-up
+  interrupt may be specified.
 - interrupt-controller: Marks the device node as an interrupt controller.
 - #interrupt-cells : Should be 2.
   The first cell is the GPIO number.
-- 
2.17.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox