* Re: [PATCH V7 3/3] dt-bindings: serial: Add binding for UART pin swap
From: Rob Herring @ 2020-05-29 17:57 UTC (permalink / raw)
To: Akash Asthana
Cc: robh+dt, rojay, linux-kernel, msavaliy, mgautam, skakit,
devicetree, linux-arm-msm
In-Reply-To: <1590560864-27037-4-git-send-email-akashast@codeaurora.org>
On Wed, 27 May 2020 11:57:44 +0530, Akash Asthana wrote:
> Add documentation to support RX-TX & CTS-RTS GPIO pin swap in HW.
>
> Signed-off-by: Akash Asthana <akashast@codeaurora.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> ---
> Changes in V7:
> - As per Rob's comment, added type: boolean to properties.
>
> Documentation/devicetree/bindings/serial/serial.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH] ARM: dts: imx53: ppd: alarm LEDs use kernel LED interface
From: Sebastian Reichel @ 2020-05-29 18:03 UTC (permalink / raw)
To: Pavel Machek
Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Rob Herring, devicetree, linux-kernel, kernel,
Ian Ray, Samu Nuutamo
In-Reply-To: <20200529160204.GA6025@duo.ucw.cz>
[-- Attachment #1: Type: text/plain, Size: 2664 bytes --]
Hi,
On Fri, May 29, 2020 at 06:02:04PM +0200, Pavel Machek wrote:
> > ping?
>
> Well, I thought that we maybe do not need standard LEDs on medical hardware.
The discussion died and the patch was not applied :) In general
IDK how worthwhile it is to use standard LED names for them. I
suppose the number of people planning to create something like
OpenWRT for medical devices is not so big.
> > On Fri, Apr 24, 2020 at 02:44:23PM +0200, Sebastian Reichel wrote:
> > > On Fri, Apr 24, 2020 at 11:32:26AM +0200, Pavel Machek wrote:
> > > > On Thu 2020-04-16 16:51:23, Sebastian Reichel wrote:
> > > > > From: Ian Ray <ian.ray@ge.com>
> > > > >
> > > > > Use kernel LED interface for the alarm LEDs.
> > > >
> > > > Could we get these changes cced to LED maintainers?
> > >
> > > Sorry, you are not turning up via get_maintainer.pl and usually
> > > subsystem maintainers are not CC'd for every DT device instance.
> > > E.g. I do not want to be always CC'd for DT board file containing
> > > a battery/charger. I'm quite surprised you want to be CC'd for
> > > them, just looking at ARM DT files there are over 1000 instances
> > > of leds.
>
> Well, we have mess in the naming; I'd like to clear it up.
I understand.
> > > > > + alarm1 {
> > > > > + label = "alarm:red";
> > > > > + gpios = <&gpio7 3 GPIO_ACTIVE_HIGH>;
> > > > > + };
> > > >
> > > > So... What is function of these leds, and can we get naming more
> > > > consistent with rest of the kernel?
> > >
> > > The device is a medical patient monitor and these are alarm LEDs
> > > informing about critical device or patient status. They are
> > > referenced by their color (those are discrete LEDs, not a
> > > multi-color one) basically everywhere. The only exception is
> > > "silenced", which means that audible alarm is surpressed. I
> > > don't think we have something comparable for any of those LEDs
> > > in the mainline tree.
>
> Actually, we have "platform:*:mute" LEDs, that could be used for
> "silenced".
I see you point, but wonder if mute is the right choice. The LED
signals a silenced alarm, which IMHO is not the same:
* The alarm silencing is temporary and system unsilences after
1-2 minutes.
* LED is usually blinking instead of solid like a laptop mute LED
(so that operator is aware of silenced alarm)
* Device usually cannot be put into silenced mode before the alarm
appears
* Some medical devices still generate perodic beeps
AFAIK this is named alarm silencing by basically everyone for
medical devices. So I think naming this platfrom:*:mute would
increase the mess.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/4] dt-bindings: usb: Add documentation for SG trb cache size quirk
From: Rob Herring @ 2020-05-29 18:05 UTC (permalink / raw)
To: Tejas Joglekar; +Cc: Greg Kroah-Hartman, linux-usb, devicetree, John Youn
In-Reply-To: <d91b768b3827fce611ba052aa1bcca19ac09fd75.1590415123.git.joglekar@synopsys.com>
On Wed, May 27, 2020 at 04:10:55PM +0530, Tejas Joglekar wrote:
> This commit adds the documentation for sgl-trb-cache-size-quirk, and
> snps,sgl-trb-cache-size-quirk property. These when set enables the
> quirk for XHCI driver for consolidation of sg list into a temporary
> buffer when small buffer sizes are scattered over the sg list not
> making up to MPS or total transfer size within TRB cache size with
> Synopsys xHC.
>
> Signed-off-by: Tejas Joglekar <joglekar@synopsys.com>
> ---
> Documentation/devicetree/bindings/usb/dwc3.txt | 4 ++++
> Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++
> 2 files changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> index d03edf9d3935..0fcbaa51f66e 100644
> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> @@ -102,6 +102,10 @@ Optional properties:
> this and tx-thr-num-pkt-prd to a valid, non-zero value
> 1-16 (DWC_usb31 programming guide section 1.2.3) to
> enable periodic ESS TX threshold.
> + - snps,sgl-trb-cache-size-quirk: enable sg list consolidation - host mode
> + only. Set to use SG buffers of at least MPS size
> + by consolidating smaller SG buffers list into a
> + single buffer.
>
> - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated.
> - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0
> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> index dc025f126d71..c53eb19ae67e 100644
> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> @@ -44,6 +44,9 @@ Optional properties:
> - quirk-broken-port-ped: set if the controller has broken port disable mechanism
> - imod-interval-ns: default interrupt moderation interval is 5000ns
> - phys : see usb-hcd.yaml in the current directory
> + - sgl-trb-cache-size-quirk: set if you need to consolidate sg list into a
> + temporary buffer when small SG buffer sizes does not make upto MPS
> + size or total transfer size across the TRB cache size.
Still don't understand why you have 2 properties? Is this a generic
issue for multiple XHCI controllers? If yes, you don't need the first
one. If no, then you don't need the second one.
Really, I'd prefer neither, and this should be implied by a specific
compatible string. Having a separate property doesn't work if you find
this issue later on after already adding XHCI support. IOW, don't make
users update their DT to handle a quirk.
Rob
^ permalink raw reply
* [PATCH] dt-bindings: Merge gpio-usb-b-connector with usb-connector
From: Thierry Reding @ 2020-05-29 18:06 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Prashant Malani, devicetree, linux-usb,
linux-kernel
From: Thierry Reding <treding@nvidia.com>
The binding for usb-connector is a superset of gpio-usb-b-connector. One
major difference is that gpio-usb-b-connector requires at least one of
the vbus-gpios and id-gpios properties to be specified. Merge the two
bindings by adding the compatible string combination for the GPIO USB-B
variant and an extra conditional for the required properties list to the
usb-connector.yaml file.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
.../bindings/connector/usb-connector.yaml | 39 +++++++++++++++++--
.../devicetree/bindings/usb/usb-conn-gpio.txt | 30 --------------
2 files changed, 35 insertions(+), 34 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/usb/usb-conn-gpio.txt
diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index 03b92b6f35fa..9bd52e63c935 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -15,10 +15,15 @@ description:
properties:
compatible:
- enum:
- - usb-a-connector
- - usb-b-connector
- - usb-c-connector
+ oneOf:
+ - enum:
+ - usb-a-connector
+ - usb-b-connector
+ - usb-c-connector
+
+ - items:
+ - const: gpio-usb-b-connector
+ - const: usb-b-connector
label:
description: Symbolic name for the connector.
@@ -140,6 +145,19 @@ properties:
required:
- compatible
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: gpio-usb-b-connector
+ then:
+ anyOf:
+ - required:
+ - vbus-gpios
+ - required:
+ - id-gpios
+
examples:
# Micro-USB connector with HS lines routed via controller (MUIC).
- |
@@ -202,3 +220,16 @@ examples:
op-sink-microwatt = <10000000>;
};
};
+
+ # USB connector with GPIO control lines
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ usb {
+ connector {
+ compatible = "gpio-usb-b-connector", "usb-b-connector";
+ type = "micro";
+ id-gpios = <&pio 12 GPIO_ACTIVE_HIGH>;
+ vbus-supply = <&usb_p0_vbus>;
+ };
+ };
diff --git a/Documentation/devicetree/bindings/usb/usb-conn-gpio.txt b/Documentation/devicetree/bindings/usb/usb-conn-gpio.txt
deleted file mode 100644
index ec80641208a5..000000000000
--- a/Documentation/devicetree/bindings/usb/usb-conn-gpio.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-USB GPIO Based Connection Detection
-
-This is typically used to switch dual role mode from the USB ID pin connected
-to an input GPIO, and also used to enable/disable device mode from the USB
-Vbus pin connected to an input GPIO.
-
-Required properties:
-- compatible : should include "gpio-usb-b-connector" and "usb-b-connector".
-- id-gpios, vbus-gpios : input gpios, either one of them must be present,
- and both can be present as well.
- see connector/usb-connector.yaml
-
-Optional properties:
-- vbus-supply : can be present if needed when supports dual role mode.
- see connector/usb-connector.yaml
-
-- Sub-nodes:
- - port : can be present.
- see graph.txt
-
-Example:
-
-&mtu3 {
- connector {
- compatible = "gpio-usb-b-connector", "usb-b-connector";
- type = "micro";
- id-gpios = <&pio 12 GPIO_ACTIVE_HIGH>;
- vbus-supply = <&usb_p0_vbus>;
- };
-};
--
2.24.1
^ permalink raw reply related
* Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
From: Pavel Machek @ 2020-05-29 18:07 UTC (permalink / raw)
To: Martin Kepplinger
Cc: robh, kernel, shawnguo, s.hauer, kernel, festevam, linux-imx,
mchehab, Anson.Huang, agx, angus, linux-kernel, devicetree,
linux-arm-kernel
In-Reply-To: <20200529162850.GC3709@amd>
Hi!
Plus, do we need calibration matrix for magnetometer?
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: [PATCH V6 1/5] dt-bindings: clock: add ipq6018 a53 pll compatible
From: Rob Herring @ 2020-05-29 18:08 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: agross, bjorn.andersson, mturquette, sboyd, linux-arm-msm,
linux-clk, devicetree, linux-kernel
In-Reply-To: <1590582292-13314-2-git-send-email-sivaprak@codeaurora.org>
On Wed, May 27, 2020 at 05:54:48PM +0530, Sivaprakash Murugesan wrote:
> cpus on ipq6018 are clocked by a53 pll, add device compatible for a53
> pll found on ipq6018 devices.
>
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> * [V6]
> re-ordered compatible string, dropped Rob's review tag for this change.
Not really significant enough to drop it, but if you really want me to
stare at this again...
> .../devicetree/bindings/clock/qcom,a53pll.yaml | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> index 20d2638..a4f2d01 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> @@ -15,6 +15,7 @@ description:
>
> properties:
> compatible:
> + const: qcom,ipq6018-a53pll
> const: qcom,msm8916-a53pll
>
> reg:
> @@ -23,6 +24,14 @@ properties:
> '#clock-cells':
> const: 0
>
> + clocks:
> + items:
> + - description: board XO clock
> +
> + clock-names:
> + items:
> + - const: xo
> +
> required:
> - compatible
> - reg
> @@ -38,3 +47,12 @@ examples:
> reg = <0xb016000 0x40>;
> #clock-cells = <0>;
> };
> + #Example 2 - A53 PLL found on IPQ6018 devices
> + - |
> + a53pll_ipq: clock@b116000 {
clock-controller@...
> + compatible = "qcom,ipq6018-a53pll";
> + reg = <0x0b116000 0x40>;
> + #clock-cells = <0>;
> + clocks = <&xo>;
> + clock-names = "xo";
> + };
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH V6 3/5] clk: qcom: Add DT bindings for ipq6018 apss clock controller
From: Rob Herring @ 2020-05-29 18:08 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: sboyd, robh+dt, agross, mturquette, linux-clk, linux-kernel,
linux-arm-msm, devicetree, bjorn.andersson
In-Reply-To: <1590582292-13314-4-git-send-email-sivaprak@codeaurora.org>
On Wed, 27 May 2020 17:54:50 +0530, Sivaprakash Murugesan wrote:
> Add dt-binding for ipq6018 apss clock controller
>
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> [V6]
> * Addressed review comment from Stephen
> include/dt-bindings/clock/qcom,apss-ipq.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
> create mode 100644 include/dt-bindings/clock/qcom,apss-ipq.h
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox