* Re: [PATCH v25 03/16] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
From: Rob Herring @ 2020-05-27 1:59 UTC (permalink / raw)
To: Dan Murphy; +Cc: jacek.anaszewski, pavel, devicetree, linux-leds, linux-kernel
In-Reply-To: <20200526164652.2331-4-dmurphy@ti.com>
On Tue, May 26, 2020 at 11:46:39AM -0500, Dan Murphy wrote:
> Introduce the bindings for the Texas Instruments LP5036, LP5030, LP5024,
> LP5018, LP5012 and LP5009 RGB LED device driver. The LP5036/30/24/18/12/9
> can control RGB LEDs individually or as part of a control bank group.
> These devices have the ability to adjust the mixing control for the RGB
> LEDs to obtain different colors independent of the overall brightness of
> the LED grouping.
>
> Datasheet:
> http://www.ti.com/lit/ds/symlink/lp5012.pdf
> http://www.ti.com/lit/ds/symlink/lp5024.pdf
> http://www.ti.com/lit/ds/symlink/lp5036.pdf
>
> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> .../devicetree/bindings/leds/leds-lp50xx.yaml | 180 ++++++++++++++++++
> 1 file changed, 180 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
> new file mode 100644
> index 000000000000..a2ea03e07f6d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
> @@ -0,0 +1,180 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/leds-lp50xx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LED driver for LP50XX RGB LED from Texas Instruments.
> +
> +maintainers:
> + - Dan Murphy <dmurphy@ti.com>
> +
> +description: |
> + The LP50XX is multi-channel, I2C RGB LED Drivers that can group RGB LEDs into
> + a LED group or control them individually.
> +
> + The difference in these RGB LED drivers is the number of supported RGB
> + modules.
> +
> + For more product information please see the link below:
> + http://www.ti.com/lit/ds/symlink/lp5012.pdf
> + http://www.ti.com/lit/ds/symlink/lp5024.pdf
> + http://www.ti.com/lit/ds/symlink/lp5036.pdf
> +
> +properties:
> + compatible:
> + oneOf:
> + - const: ti,lp5009
> + - const: ti,lp5012
> + - const: ti,lp5018
> + - const: ti,lp5024
> + - const: ti,lp5030
> + - const: ti,lp5036
Use enum rather than oneOf+const.
> +
> + reg:
> + maxItems: 1
> + description:
> + I2C slave address
> + lp5009/12 - 0x14, 0x15, 0x16, 0x17
> + lp5018/24 - 0x28, 0x29, 0x2a, 0x2b
> + lp5030/36 - 0x30, 0x31, 0x32, 0x33
> +
> + enable-gpios:
> + description: GPIO pin to enable/disable the device.
How many? (maxItems: 1)
> +
> + vled-supply:
> + description: LED supply.
> +
> + child-node:
This literally requires a node called 'child-node'. Not what you want.
You need a $ref to the multi-color schema in here and then only define
what's specific to this chip.
> + type: object
> + properties:
> + reg:
> + description: This is the LED module number.
Constraints?
> +
> + color:
> + description: Must be LED_COLOR_ID_MULTI
> +
> + function:
> + description: see Documentation/devicetree/bindings/leds/common.txt
> +
> + ti,led-bank:
> + description:
> + This property denotes the LED module numbers that will be controlled as
> + a single RGB cluster. Each LED module number will be controlled by a
> + single LED class instance.
> + There can only be one instance of the ti,led-bank
> + property for each device node. This is a required node is the LED
> + modules are to be backed.
> + $ref: /schemas/types.yaml#definitions/uint32-array
What is reg then? Some made up index? Can't you do:
reg = <1 2 3>;
led@1 {};
led@2 {};
led@2 {};
> +
> + required:
> + - reg
> + - color
> + - function
> +
> + grandchild-node:
Again, no.
> + type: object
> + properties:
> + reg:
> + description:
> + A single entry denoting the LED output that controls the monochrome LED.
Constraints?
> +
> + color:
> + description:
> + see Documentation/devicetree/bindings/leds/common.txt
Have you read this file recently? Don't add new references to it. (And
generally freeform references to other files are wrong with schemas).
> +
> + led-sources:
> + description:
> + see Documentation/devicetree/bindings/leds/common.txt
> + The LED outputs associated with the LED modules are defined in Table 1
> + of the corresponding data sheets.
> + LP5009 - 3 Total RGB cluster LED outputs 0-2
> + LP5012 - 4 Total RGB cluster LED outputs 0-3
> + LP5018 - 6 Total RGB cluster LED outputs 0-5
> + LP5024 - 8 Total RGB cluster LED outputs 0-7
> + LP5030 - 10 Total RGB cluster LED outputs 0-9
> + LP5036 - 12 Total RGB cluster LED outputs 0-11
> +
> + label:
> + description: |
> + Optional node - see Documentation/devicetree/bindings/leds/common.txt
> +
> + linux,default-trigger:
> + description: |
> + Optional node - see Documentation/devicetree/bindings/leds/common.txt
> +
> + required:
> + - reg
> + - color
> +
> +required:
> + - compatible
> + - reg
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/leds/common.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led-controller@14 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "ti,lp5009";
> + reg = <0x14>;
> + enable-gpios = <&gpio1 16>;
> + multi-led@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> + color = <LED_COLOR_ID_MULTI>;
> + function = LED_FUNCTION_CHARGING;
> +
> + led@0 {
> + reg = <0>;
> + color = <LED_COLOR_ID_RED>;
> + };
> +
> + led@1 {
> + reg = <1>;
> + color = <LED_COLOR_ID_GREEN>;
> + };
> +
> + led@2 {
> + reg = <2>;
> + color = <LED_COLOR_ID_BLUE>;
> + };
> + };
> +
> + multi-led@2 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <2>;
> + color = <LED_COLOR_ID_MULTI>;
> + function = LED_FUNCTION_STANDBY;
> + ti,led-bank = <2 3 5>;
> +
> + led@6 {
> + reg = <0x6>;
> + color = <LED_COLOR_ID_RED>;
> + led-sources = <6 9 15>;
> + };
> +
> + led@7 {
> + reg = <0x7>;
> + color = <LED_COLOR_ID_GREEN>;
> + led-sources = <7 10 16>;
> + };
> +
> + led@8 {
> + reg = <0x8>;
> + color = <LED_COLOR_ID_BLUE>;
> + led-sources = <8 11 17>;
> + };
> + };
> + };
> + };
> +
> +...
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH v8 06/14] media: platform: Improve the implementation of the system PM ops
From: Xia Jiang @ 2020-05-27 1:52 UTC (permalink / raw)
To: Tomasz Figa
Cc: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring,
Matthias Brugger, Rick Chang, linux-media, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek, Marek Szyprowski,
srv_heupstream, senozhatsky, mojahsu, drinkcat, maoguang.meng,
sj.huang
In-Reply-To: <20200521153257.GF209565@chromium.org>
On Thu, 2020-05-21 at 15:32 +0000, Tomasz Figa wrote:
> Hi Xia,
>
> On Fri, Apr 03, 2020 at 05:40:25PM +0800, Xia Jiang wrote:
> > Cancel reset hw operation in suspend and resume function because this
> > will be done in device_run().
>
> This and...
>
> > Add spin_lock and unlock operation in irq and resume function to make
> > sure that the current frame is processed completely before suspend.
>
> ...this are two separate changes. Please split.
>
> >
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index dd5cadd101ef..2fa3711fdc9b 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -911,6 +911,8 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> > u32 dec_ret;
> > int i;
> >
> > + spin_lock(&jpeg->hw_lock);
> > +
>
> nit: For consistency, it is recommended to always use the same, i.e. the
> strongest, spin_(un)lock_ primitives when operating on the same spinlock.
> In this case it would be the irqsave(restore) variants.
>
> > dec_ret = mtk_jpeg_dec_get_int_status(jpeg->dec_reg_base);
> > dec_irq_ret = mtk_jpeg_dec_enum_result(dec_ret);
> > ctx = v4l2_m2m_get_curr_priv(jpeg->m2m_dev);
> > @@ -941,6 +943,7 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> > v4l2_m2m_buf_done(src_buf, buf_state);
> > v4l2_m2m_buf_done(dst_buf, buf_state);
> > v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > + spin_unlock(&jpeg->hw_lock);
> > pm_runtime_put_sync(ctx->jpeg->dev);
> > return IRQ_HANDLED;
> > }
> > @@ -1191,7 +1194,6 @@ static __maybe_unused int mtk_jpeg_pm_suspend(struct device *dev)
> > {
> > struct mtk_jpeg_dev *jpeg = dev_get_drvdata(dev);
> >
> > - mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> > mtk_jpeg_clk_off(jpeg);
> >
> > return 0;
> > @@ -1202,19 +1204,24 @@ static __maybe_unused int mtk_jpeg_pm_resume(struct device *dev)
> > struct mtk_jpeg_dev *jpeg = dev_get_drvdata(dev);
> >
> > mtk_jpeg_clk_on(jpeg);
> > - mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> >
> > return 0;
> > }
> >
> > static __maybe_unused int mtk_jpeg_suspend(struct device *dev)
> > {
> > + struct mtk_jpeg_dev *jpeg = dev_get_drvdata(dev);
> > + unsigned long flags;
> > int ret;
> >
> > if (pm_runtime_suspended(dev))
> > return 0;
> >
> > + spin_lock_irqsave(&jpeg->hw_lock, flags);
>
> What does this spinlock protect us from? I can see that it would prevent
> the interrupt handler from being called, but is it okay to suspend the
> system without handling the interrupt?
Dear Tomasz,
I mean that if current image is processed in irq handler,suspend
function can not get the lock(it was locked in irq handler).Should I
move the spin_lock_irqsave(&jpeg->hw_lock, flags) to the start location
of suspend function or use wait_event_timeout() to handle the interrupt
before suspend?
Best Regards,
Xia Jiang
>
> > +
> > ret = mtk_jpeg_pm_suspend(dev);
> > +
>
> Looking at the implementation of mtk_jpeg_pm_suspend(), all it does is
> disabling the clock. How do we make sure that there is no frame currently
> being processed by the hardware?
>
> Best regards,
> Tomasz
^ permalink raw reply
* Re: [PATCH v25 01/16] dt: bindings: Add multicolor class dt bindings documention
From: Rob Herring @ 2020-05-27 1:49 UTC (permalink / raw)
To: Dan Murphy; +Cc: jacek.anaszewski, pavel, devicetree, linux-leds, linux-kernel
In-Reply-To: <20200526164652.2331-2-dmurphy@ti.com>
On Tue, May 26, 2020 at 11:46:37AM -0500, Dan Murphy wrote:
> Add DT bindings for the LEDs multicolor class framework.
> Add multicolor ID to the color ID list for device tree bindings.
>
> CC: Rob Herring <robh@kernel.org>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> .../bindings/leds/leds-class-multicolor.yaml | 71 +++++++++++++++++++
> drivers/leds/led-core.c | 1 +
> include/dt-bindings/leds/common.h | 3 +-
> 3 files changed, 74 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml
> new file mode 100644
> index 000000000000..fa6ea8e5c46b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: GPL-2.0
Dual license new bindings please.
GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/leds-class-multicolor.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common properties for the multicolor LED class.
> +
> +maintainers:
> + - Dan Murphy <dmurphy@ti.com>
> +
> +description: |
> + Bindings for multi color LEDs show how to describe current outputs of
> + either integrated multi-color LED elements (like RGB, RGBW, RGBWA-UV
> + etc.) or standalone LEDs, to achieve logically grouped multi-color LED
> + modules. This is achieved by adding multi-led nodes layer to the
> + monochrome LED bindings.
> + The nodes and properties defined in this document are unique to the multicolor
> + LED class. Common LED nodes and properties are inherited from the common.txt
> + within this documentation directory.
> +
> +properties:
> + color:
> + description: |
> + For multicolor LED support this property should be defined as
> + LED_COLOR_ID_MULTI and further definition can be found in
> + include/linux/leds/common.h.
This should have:
const: X # LED_COLOR_ID_MULTI
It can't use the define, so you have to use the value.
> +
> +required:
> + - color
> +
> +examples:
> + - |
> + #include <dt-bindings/leds/common.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led-controller@14 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "ti,lp5009";
> + reg = <0x14>;
> +
> + multi-led@1 {
Define $nodename must be 'multi-led(@[0-9a-f]+)?'
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> + color = <LED_COLOR_ID_MULTI>;
> + function = LED_FUNCTION_CHARGING;
> +
> + led@0 {
Then the schema should define child nodes named 'led'.
> + reg = <0>;
> + color = <LED_COLOR_ID_RED>;
I assume in this case 'reg' and 'color' need to be defined as required?
> + };
> +
> + led@1 {
> + reg = <1>;
> + color = <LED_COLOR_ID_GREEN>;
> + };
> +
> + led@2 {
> + reg = <2>;
> + color = <LED_COLOR_ID_BLUE>;
> + };
> + };
> + };
> + };
> +
> +additionalProperties: false
> +...
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
This isn't a binding file. Belongs in another patch.
> index f1f718dbe0f8..846248a0693d 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -34,6 +34,7 @@ const char * const led_colors[LED_COLOR_ID_MAX] = {
> [LED_COLOR_ID_VIOLET] = "violet",
> [LED_COLOR_ID_YELLOW] = "yellow",
> [LED_COLOR_ID_IR] = "ir",
> + [LED_COLOR_ID_MULTI] = "multicolor",
> };
> EXPORT_SYMBOL_GPL(led_colors);
>
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> index 0ce7dfc00dcb..a463ce6a8794 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -30,7 +30,8 @@
> #define LED_COLOR_ID_VIOLET 5
> #define LED_COLOR_ID_YELLOW 6
> #define LED_COLOR_ID_IR 7
> -#define LED_COLOR_ID_MAX 8
> +#define LED_COLOR_ID_MULTI 8
> +#define LED_COLOR_ID_MAX 9
>
> /* Standard LED functions */
> /* Keyboard LEDs, usually it would be input4::capslock etc. */
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: clk: intel: Add bindings document & header file for CGU
From: Stephen Boyd @ 2020-05-27 1:48 UTC (permalink / raw)
To: Rahul Tanwar, linux-clk, mturquette
Cc: robh, mark.rutland, linux-kernel, devicetree, andriy.shevchenko,
qi-ming.wu, yixin.zhu, cheol.yong.kim, Rahul Tanwar
In-Reply-To: <8dce2be13195aab20c6b11fca6af0fffe22d5241.1587102634.git.rahul.tanwar@linux.intel.com>
Quoting Rahul Tanwar (2020-04-16 22:54:46)
> Clock generation unit(CGU) is a clock controller IP of Intel's Lightning
> Mountain(LGM) SoC. Add DT bindings include file and document for CGU clock
> controller driver of LGM.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Rahul Tanwar <rahul.tanwar@linux.intel.com>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v5 5/8] clk: stm32: Fix stm32f429's ltdc driver hang in set clock rate, fix duplicated ltdc clock register to 'clk_core' case ltdc's clock turn off by clk_disable_unused()
From: Stephen Boyd @ 2020-05-27 1:44 UTC (permalink / raw)
To: broonie, dillon.minfei, linus.walleij
Cc: devicetree, linux-arm-kernel, linux-kernel, linux-spi,
linux-stm32, dri-devel, linux-clk, dillon min
In-Reply-To: <1590378348-8115-6-git-send-email-dillon.minfei@gmail.com>
Quoting dillon.minfei@gmail.com (2020-05-24 20:45:45)
> From: dillon min <dillon.minfei@gmail.com>
>
> ltdc set clock rate crashed
> 'post_div_data[]''s pll_num is PLL_I2S, PLL_SAI (number is 1,2). but,
Please write "post_div_data[]'s" if it is possessive. "But" doesn't
start a sentence. This is one sentence, not two.
> as pll_num is offset of 'clks[]' input to clk_register_pll_div(), which
> is FCLK, CLK_LSI, defined in 'include/dt-bindings/clock/stm32fx-clock.h'
> so, this is a null object at the register time.
> then, in ltdc's clock is_enabled(), enable(), will call to_clk_gate().
> will return a null object, cause kernel crashed.
> need change pll_num to PLL_VCO_I2S, PLL_VCO_SAI for 'post_div_data[]'
>
> duplicated ltdc clock
> 'stm32f429_gates[]' has a member 'ltdc' register to 'clk_core', but no
> upper driver use it, ltdc driver use the lcd-tft defined in
> 'stm32f429_aux_clk[]'. after system startup, as stm32f429_gates[]'s ltdc
> enable_count is zero, so turn off by clk_disable_unused()
I sort of follow this. Is this another patch? Seems like two things are
going on here.
>
> Changes since V3:
> 1 drop last wrong changes about 'CLK_IGNORE_UNUSED' patch
> 2 fix PLL_SAI mismatch with PLL_VCO_SAI
This change log goes under the --- below.
>
> Signed-off-by: dillon min <dillon.minfei@gmail.com>
Any Fixes tag?
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: usb: convert keystone-usb.txt to YAML
From: Rob Herring @ 2020-05-27 1:37 UTC (permalink / raw)
To: Roger Quadros; +Cc: balbi, vigneshr, linux-usb, devicetree, linux-kernel
In-Reply-To: <20200513130709.10239-2-rogerq@ti.com>
On Wed, May 13, 2020 at 04:07:07PM +0300, Roger Quadros wrote:
> Convert keystone-usb documentation to YAML format.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> .../devicetree/bindings/usb/keystone-usb.txt | 56 ----------------
> .../bindings/usb/ti,keystone-dwc3.yaml | 67 +++++++++++++++++++
> 2 files changed, 67 insertions(+), 56 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/usb/keystone-usb.txt
> create mode 100644 Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
>
> diff --git a/Documentation/devicetree/bindings/usb/keystone-usb.txt b/Documentation/devicetree/bindings/usb/keystone-usb.txt
> deleted file mode 100644
> index 77df82e36138..000000000000
> --- a/Documentation/devicetree/bindings/usb/keystone-usb.txt
> +++ /dev/null
> @@ -1,56 +0,0 @@
> -TI Keystone Soc USB Controller
> -
> -DWC3 GLUE
> -
> -Required properties:
> - - compatible: should be
> - "ti,keystone-dwc3" for Keystone 2 SoCs
> - "ti,am654-dwc3" for AM654 SoC
> - - #address-cells, #size-cells : should be '1' if the device has sub-nodes
> - with 'reg' property.
> - - reg : Address and length of the register set for the USB subsystem on
> - the SOC.
> - - interrupts : The irq number of this device that is used to interrupt the
> - MPU.
> - - ranges: allows valid 1:1 translation between child's address space and
> - parent's address space.
> -
> -SoC-specific Required Properties:
> -The following are mandatory properties for Keystone 2 66AK2HK, 66AK2L and 66AK2E
> -SoCs only:
> -
> -- clocks: Clock ID for USB functional clock.
> -- clock-names: Must be "usb".
> -
> -
> -The following are mandatory properties for 66AK2G and AM654:
> -
> -- power-domains: Should contain a phandle to a PM domain provider node
> - and an args specifier containing the USB device id
> - value. This property is as per the binding,
> - Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> -
> -Sub-nodes:
> -The dwc3 core should be added as subnode to Keystone DWC3 glue.
> -- dwc3 :
> - The binding details of dwc3 can be found in:
> - Documentation/devicetree/bindings/usb/dwc3.txt
> -
> -Example:
> - usb: usb@2680000 {
> - compatible = "ti,keystone-dwc3";
> - #address-cells = <1>;
> - #size-cells = <1>;
> - reg = <0x2680000 0x10000>;
> - clocks = <&clkusb>;
> - clock-names = "usb";
> - interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> - ranges;
> -
> - dwc3@2690000 {
> - compatible = "synopsys,dwc3";
> - reg = <0x2690000 0x70000>;
> - interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> - usb-phy = <&usb_phy>, <&usb_phy>;
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
> new file mode 100644
> index 000000000000..14d2fe329b93
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/ti,keystone-dwc3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI Keystone Soc USB Controller
> +
> +maintainers:
> + - Roger Quadros <rogerq@ti.com>
> +
> +properties:
> + compatible:
> + oneOf:
> + - const: "ti,keystone-dwc3"
> + - const: "ti,am654-dwc3"
Use enum rather than oneOf+const.
> +
> + reg:
> + maxItems: 1
> + description: Address and length of the register set for the USB subsystem on
> + the SOC.
> +
> + interrupts:
> + maxItems: 1
> + description: The irq number of this device that is used to interrupt the MPU.
No need for genericish descriptions when a single item.
> +
> +
> + clocks:
> + description: Clock ID for USB functional clock.
How many?
> +
> + power-domains:
> + description: Should contain a phandle to a PM domain provider node
> + and an args specifier containing the USB device id
> + value. This property is as per the binding,
> + Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
How many?
> +
> + dwc3:
This doesn't work because there's a unit address. You need a pattern.
> + description: This is the node representing the DWC3 controller instance
> + Documentation/devicetree/bindings/usb/dwc3.txt
type: object
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + usb: usb@2680000 {
> + compatible = "ti,keystone-dwc3";
> + #address-cells = <1>;
> + #size-cells = <1>;
These have to be documented.
> + reg = <0x2680000 0x10000>;
> + clocks = <&clkusb>;
> + clock-names = "usb";
> + interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> + ranges;
This too.
> +
> + dwc3@2690000 {
> + compatible = "synopsys,dwc3";
> + reg = <0x2690000 0x70000>;
> + interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> + usb-phy = <&usb_phy>, <&usb_phy>;
> + };
> + };
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
^ permalink raw reply
* Re: [PATCH 16/17] dt-bindings: watchdog: renesas,wdt: Document r8a7742 support
From: Rob Herring @ 2020-05-27 1:31 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Geert Uytterhoeven, Jens Axboe, Wolfram Sang, Ulf Hansson,
Sergei Shtylyov, David S. Miller, Wim Van Sebroeck, Guenter Roeck,
linux-ide, devicetree, linux-kernel, linux-i2c, linux-mmc, netdev,
linux-renesas-soc, linux-watchdog, Prabhakar
In-Reply-To: <1589555337-5498-17-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Fri, May 15, 2020 at 04:08:56PM +0100, Lad Prabhakar wrote:
> RZ/G1H (R8A7742) watchdog implementation is compatible with R-Car Gen2,
> therefore add relevant documentation.
>
> 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/watchdog/renesas,wdt.txt | 1 +
> 1 file changed, 1 insertion(+)
Meanwhile in the DT tree, converting this schema landed. Can you prepare
a version based on the schema.
Rob
^ permalink raw reply
* Re: [PATCH v3 02/12] dt-bindings: i2c: Convert DW I2C binding to DT schema
From: Rob Herring @ 2020-05-27 1:20 UTC (permalink / raw)
To: Serge Semin
Cc: linux-i2c, Wolfram Sang, devicetree, Mika Westerberg, Serge Semin,
Rob Herring, Jarkko Nikula, Thomas Bogendoerfer, Andy Shevchenko,
linux-kernel, linux-mips, Alexey Malahov
In-Reply-To: <20200526215528.16417-3-Sergey.Semin@baikalelectronics.ru>
On Wed, 27 May 2020 00:55:18 +0300, Serge Semin wrote:
> Modern device tree bindings are supposed to be created as YAML-files
> in accordance with dt-schema. This commit replaces Synopsys DW I2C
> 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 I2C controller or with Microsemi Ocelot SoC I2C
> one, to have registers, interrupts and clocks properties. In addition
> the node may have clock-frequency, i2c-sda-hold-time-ns,
> i2c-scl-falling-time-ns and i2c-sda-falling-time-ns optional properties.
>
> 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 v2:
> - Make sure that "mscc,ocelot-i2c" compatible node may have up to two
> registers space defined, while normal DW I2C controller will have only
> one registers space.
> - Add "mscc,ocelot-i2c" example to test the previous fix.
> - Declare "unevaluatedProperties" property instead of
> "additionalProperties" one.
> - Due to the previous fix we can now discard the dummy boolean properties
> definitions, since the proper type evaluation will be performed by the
> generic i2c-controller.yaml schema.
>
> Changelog v3:
> - Discard $ref from the "-ns" suffixed properties since they've got the
> uint32-array type by default applied in the common schema. Set "maxItems: 1"
> there instead to make sure the property will have a single value specified.
> ---
> .../bindings/i2c/i2c-designware.txt | 73 ---------
> .../bindings/i2c/snps,designware-i2c.yaml | 154 ++++++++++++++++++
> 2 files changed, 154 insertions(+), 73 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-designware.txt
> create mode 100644 Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 01/12] scripts/dtc: check: Add 10bit/slave i2c reg flags support
From: Rob Herring @ 2020-05-27 1:17 UTC (permalink / raw)
To: Serge Semin
Cc: Jarkko Nikula, Wolfram Sang, Frank Rowand, Serge Semin,
Alexey Malahov, Thomas Bogendoerfer, Mika Westerberg, linux-mips,
linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200526215528.16417-2-Sergey.Semin@baikalelectronics.ru>
On Wed, May 27, 2020 at 12:55:17AM +0300, Serge Semin wrote:
> Recently the I2C-controllers slave interface support was added to the
> kernel I2C subsystem. In this case Linux can be used as, for example,
> a I2C EEPROM machine. See [1] for details. Other than instantiating
> the EEPROM-slave device from user-space there is a way to declare the
> device in dts. In this case firstly the I2C bus controller must support
> the slave interface. Secondly I2C-slave sub-node of that controller
> must have "reg"-property with flag I2C_OWN_SLAVE_ADDRESS set (flag is
> declared in [2]). That flag is declared as (1 << 30), which when set
> makes dtc unhappy about too big address set for a I2C-slave:
>
> 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"
>
> Similar problem would have happened if we had set the 10-bit address
> flag I2C_TEN_BIT_ADDRESS in the "reg"-property.
>
> In order to fix the problem we suggest to alter the I2C-bus reg-check
> algorithm, so one would be aware of the upper bits set. Normally if no
> flag specified, the 7-bit address is expected in the "reg"-property.
> If I2C_TEN_BIT_ADDRESS is set, then the 10-bit address check will be
> performed. The I2C_OWN_SLAVE_ADDRESS flag will be just ignored.
>
> [1] Documentation/i2c/slave-interface.rst
> [2] include/dt-bindings/i2c/i2c.h
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: linux-mips@vger.kernel.org
> Cc: linux-i2c@vger.kernel.org
> ---
> scripts/dtc/checks.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
I've lost track of who all I've said this to already for this issue, but
patches to dtc should be against upstream and a version of this has been
sent there already. But it seems they've lost interest in addressing the
review comments. So feel free to send another one. The same comment
applies here.
Rob
^ permalink raw reply
* Re: [PATCH net-next v3 4/4] net: dp83869: Add RGMII internal delay configuration
From: Andrew Lunn @ 2020-05-27 0:54 UTC (permalink / raw)
To: Dan Murphy
Cc: f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
devicetree
In-Reply-To: <20200526174716.14116-5-dmurphy@ti.com>
> +static int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500, 1750,
> + 2000, 2250, 2500, 2750, 3000, 3250,
> + 3500, 3750, 4000};
> +
You should make this const. Otherwise it takes up twice the space.
Andrew
^ permalink raw reply
* Re: [PATCH v10 3/6] dt-bindings: clock: Add X1830 bindings.
From: Rob Herring @ 2020-05-27 0:54 UTC (permalink / raw)
To: 周琰杰 (Zhou Yanjie)
Cc: zhenwenjin, linux-clk, dongsheng.qiu, sboyd, sernia.zhou, robh+dt,
devicetree, mturquette, paul, linux-kernel, aric.pzqi
In-Reply-To: <20200526144044.71413-5-zhouyanjie@wanyeetech.com>
On Tue, 26 May 2020 22:40:41 +0800, 周琰杰 (Zhou Yanjie) wrote:
> Add the clock bindings for the X1830 Soc from Ingenic.
>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>
> Notes:
> v2->v3:
> Adjust order from [3/5] in v2 to [4/5] in v3.
>
> v3->v4:
> Adjust order from [4/5] in v3 to [3/4] in v4.
>
> v4->v5:
> Rebase on top of kernel 5.6-rc1.
>
> v5->v6:
> Add missing part of X1830's CGU.
>
> v6->v7:
> No change.
>
> v7->v8:
> Rebase on top of linux-next.
>
> v8->v9:
> No change.
>
> v9->v10:
> Add missing "X1830_CLK_TCU".
>
> .../devicetree/bindings/clock/ingenic,cgu.yaml | 2 +
> include/dt-bindings/clock/x1830-cgu.h | 55 ++++++++++++++++++++++
> 2 files changed, 57 insertions(+)
> create mode 100644 include/dt-bindings/clock/x1830-cgu.h
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next v3 4/4] net: dp83869: Add RGMII internal delay configuration
From: Andrew Lunn @ 2020-05-27 0:52 UTC (permalink / raw)
To: Dan Murphy
Cc: f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
devicetree
In-Reply-To: <20200526174716.14116-5-dmurphy@ti.com>
> @@ -218,6 +224,7 @@ static int dp83869_of_init(struct phy_device *phydev)
> ret = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_STRAP_STS1);
> if (ret < 0)
> return ret;
> +
> if (ret & DP83869_STRAP_MIRROR_ENABLED)
> dp83869->port_mirroring = DP83869_PORT_MIRRORING_EN;
> else
This random white space change does not belong in this patch.
> @@ -232,6 +239,20 @@ static int dp83869_of_init(struct phy_device *phydev)
> &dp83869->tx_fifo_depth))
> dp83869->tx_fifo_depth = DP83869_PHYCR_FIFO_DEPTH_4_B_NIB;
>
> + ret = of_property_read_u32(of_node, "rx-internal-delay-ps",
> + &dp83869->rx_id_delay);
> + if (ret) {
> + dp83869->rx_id_delay = ret;
> + ret = 0;
> + }
This looks odd.
If this optional property is not found, -EINVAL will be returned. It
could also return -ENODATA. You then assign this error value to
dp83869->rx_id_delay? I would of expected you to assign 2000, the
default value?
> +
> + ret = of_property_read_u32(of_node, "tx-internal-delay-ps",
> + &dp83869->tx_id_delay);
> + if (ret) {
> + dp83869->tx_id_delay = ret;
> + ret = 0;
> + }
> +
> return ret;
> }
> #else
> @@ -367,10 +388,45 @@ static int dp83869_configure_mode(struct phy_device *phydev,
> return ret;
> }
>
> +static int dp83869_get_delay(struct phy_device *phydev)
> +{
> + struct dp83869_private *dp83869 = phydev->priv;
> + int delay_size = ARRAY_SIZE(dp83869_internal_delay);
> + int tx_delay = 0;
> + int rx_delay = 0;
> +
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
> + tx_delay = phy_get_delay_index(phydev,
> + &dp83869_internal_delay[0],
> + delay_size, dp83869->tx_id_delay,
> + false);
> + if (tx_delay < 0) {
> + phydev_err(phydev, "Tx internal delay is invalid\n");
> + return tx_delay;
> + }
> + }
> +
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
> + rx_delay = phy_get_delay_index(phydev,
> + &dp83869_internal_delay[0],
> + delay_size, dp83869->rx_id_delay,
> + false);
> + if (rx_delay < 0) {
> + phydev_err(phydev, "Rx internal delay is invalid\n");
> + return rx_delay;
> + }
> + }
So any PHY using these properties is going to pretty much reproduce
this code. Meaning is should all be in a helper.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 2/4] net: phy: Add a helper to return the index for of the internal delay
From: Andrew Lunn @ 2020-05-27 0:42 UTC (permalink / raw)
To: Dan Murphy
Cc: f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
devicetree
In-Reply-To: <20200526174716.14116-3-dmurphy@ti.com>
> +/**
> + * phy_get_delay_index - returns the index of the internal delay
> + * @phydev: phy_device struct
> + * @delay_values: array of delays the PHY supports
> + * @size: the size of the delay array
> + * @int_delay: the internal delay to be looked up
> + * @descending: if the delay array is in descending order
> + *
> + * Returns the index within the array of internal delay passed in.
> + * Return errno if the delay is invalid or cannot be found.
> + */
> +s32 phy_get_delay_index(struct phy_device *phydev, int *delay_values, int size,
> + int int_delay, bool descending)
> +{
> + if (int_delay < 0)
> + return -EINVAL;
> +
> + if (size <= 0)
> + return -EINVAL;
> +
> + if (descending)
> + return phy_find_descending_delay(phydev, delay_values, size,
> + int_delay);
> +
> + return phy_find_ascending_delay(phydev, delay_values, size, int_delay);
> +}
> +EXPORT_SYMBOL(phy_get_delay_index);
Do we really need this ascending vs descending? This array is not
coming from device tree of anything, it is a static list in the PHY
driver. I would just define it needs to be ascending and be done.
Andrew
^ permalink raw reply
* Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings
From: Palmer Dabbelt @ 2020-05-27 0:32 UTC (permalink / raw)
To: seanga2
Cc: anup, Anup Patel, Paul Walmsley, aou, robh+dt, daniel.lezcano,
tglx, devicetree, Damien Le Moal, linux-kernel, Atish Patra,
Alistair Francis, linux-riscv
In-Reply-To: <c0e9e625-daf8-b72f-2237-06018ff5d8a0@gmail.com>
On Thu, 21 May 2020 23:29:36 PDT (-0700), seanga2@gmail.com wrote:
> On 5/22/20 1:54 AM, Anup Patel wrote:
>> On Fri, May 22, 2020 at 1:35 AM Sean Anderson <seanga2@gmail.com> wrote:
>>>
>>> On 5/21/20 9:45 AM, Anup Patel wrote:
>>>> +Required properties:
>>>> +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
>>>> + detailed implementation in case that specific bugs need to be worked around.
>>>
>>> Should the "riscv,clint0" compatible string be documented here? This
>>
>> Yes, I forgot to add this compatible string. I will add in v2.
>>
>>> peripheral is not really specific to sifive, as it is present in most
>>> rocket-chip cores.
>>
>> I agree that CLINT is present in a lot of non-SiFive RISC-V SOCs and
>> FPGAs but this IP is only documented as part of SiFive FU540 SOC.
>> (Refer, https://static.dev.sifive.com/FU540-C000-v1.0.pdf)
>>
>> The RISC-V foundation should host the CLINT spec independently
>> under https://github.com/riscv and make CLINT spec totally open.
>>
>> For now, I have documented it just like PLIC DT bindings found at:
>> Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt
>
> The PLIC seems to have its own RISC-V-sponsored documentation [1] which
> was split off from the older privileged specs. By your logic above,
> should it be renamed to riscv,plic0.txt (with a corresponding change in
> the documented compatible strings)?
>
> [1] https://github.com/riscv/riscv-plic-spec
Let's propose tagging that PLIC spec as v1.0.0 in the platform spec group, but
I don't see a reason why that wouldn't be viable. Assuming that's all OK, we
can start calling this a RISC-V PLIC (in addition to a SiFive PLIC, as they'll
be compatible).
>>
>> If RISC-V maintainers agree then I will document it as "RISC-V CLINT".
>>
>> @Palmer ?? @Paul ??
The CLINT is a SiFive spec. It has open source RTL so it's been implemented in
other designs, but it's not a RISC-V spec. The CLIC, which is a superset of
the CLINT, is a RISC-V spec. IIRC it's not finished yet (it's the fast
interrupts task group), but presumably we should have a "riscv,clic-2.0.0" (or
whatever it ends up being called) compat string to go along with the
specification.
>> Regards,
>> Anup
>>
>
> --Sean
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: clock: renesas: cpg: Convert to json-schema
From: Stephen Boyd @ 2020-05-26 23:25 UTC (permalink / raw)
To: Geert Uytterhoeven, Michael Turquette, Rob Herring
Cc: linux-renesas-soc, linux-clk, devicetree, Geert Uytterhoeven
In-Reply-To: <20200518081644.23683-1-geert+renesas@glider.be>
Quoting Geert Uytterhoeven (2020-05-18 01:16:44)
> Convert the Renesas Clock Pulse Generator (CPG) Device Tree
> binding documentation to json-schema, combining support for:
> - R-Mobile APE6 (R8A73A4) and A1 (R8A7740),
> - R-Car M1 (R8A7778) and H1 (R8A7779),
> - RZ/A1 (R7S72100),
> - SH-Mobile AG5 (SH73A0).
>
> Keep the example for R-Mobile A1, which shows most properties.
> Drop the consumer examples, as they do not belong here.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
Acked-by: Stephen Boyd <sboyd@kernel.org>
^ permalink raw reply
* Re: [RESEND v5 09/21] mtd: rawnand: Create a new enumeration to describe properly ECC types
From: Boris Brezillon @ 2020-05-26 23:21 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200527005518.2d780ecc@collabora.com>
On Wed, 27 May 2020 00:55:18 +0200
Boris Brezillon <boris.brezillon@collabora.com> wrote:
> On Tue, 26 May 2020 21:56:21 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> > Now that the misleading mix between ECC engine type and OOB placement
> > has been addressed, add a new enumeration to properly define ECC types
> > (also called provider or mode).
>
> Let's pick a name and stick to it. I think "ECC provider type" or
> "ECC engine type" are good names.
Okay, I think I remember now where the 'ECC provider' concept comes
from. IIRC, the property will be used to select one ECC engine among
possibly more than one choice: there are systems with both
on-controller and on-die ECC engines, and you can always decide to use
the SW implementation of course.
>
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> > ---
> > drivers/mtd/nand/raw/nand_base.c | 7 +++++++
> > include/linux/mtd/rawnand.h | 16 ++++++++++++++++
> > 2 files changed, 23 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> > index 515cd4681660..5c6ab5b93270 100644
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -5018,6 +5018,13 @@ static const char * const nand_ecc_modes[] = {
> > [NAND_ECC_ON_DIE] = "on-die",
> > };
> >
> > +static const char * const nand_ecc_engine_providers[] = {
>
> I'd rename that one nand_ecc_engine_types or nand_ecc_provider_types.
>
> > + [NAND_ECC_ENGINE_NONE] = "none",
> > + [NAND_ECC_ENGINE_SOFT] = "soft",
> > + [NAND_ECC_ENGINE_CONTROLLER] = "hw",
> > + [NAND_ECC_ENGINE_ON_DIE] = "on-die",
> > +};
> > +
> > static const char * const nand_ecc_placement[] = {
> > [NAND_ECC_PLACEMENT_INTERLEAVED] = "interleaved",
> > };
> > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > index dc909fb977c7..a2078c5f3d21 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -92,6 +92,22 @@ enum nand_ecc_mode {
> > NAND_ECC_ON_DIE,
> > };
> >
> > +/**
> > + * enum nand_ecc_engine_type - NAND ECC engine type/provider
> > + * @NAND_ECC_ENGINE_INVALID: Invalid value
> > + * @NAND_ECC_ENGINE_NONE: No ECC correction
> > + * @NAND_ECC_ENGINE_SOFT: Software ECC correction
> > + * @NAND_ECC_ENGINE_CONTROLLER: Hardware controller ECC correction
> > + * @NAND_ECC_ENGINE_ON_DIE: On chip hardware ECC correction
> > + */
> > +enum nand_ecc_engine_type {
>
> Looks like you went for ecc_engine_type here, so let's stick to that.
>
> > + NAND_ECC_ENGINE_INVALID,
>
> NAND_ECC_ENGINE_TYPE_xxx
>
> > + NAND_ECC_ENGINE_NONE,
>
> Do we really need a value for NONE? I'd expect the engine type to be
> applicable to NAND that have some sort of ECC engine connected to them.
>
> > + NAND_ECC_ENGINE_SOFT,
> > + NAND_ECC_ENGINE_CONTROLLER,
> > + NAND_ECC_ENGINE_ON_DIE,
> > +};
> > +
> > /**
> > * enum nand_ecc_placement - NAND ECC placement
> > * @NAND_ECC_PLACEMENT_FREE: The driver can decide where to put ECC bytes.
>
^ permalink raw reply
* Re: [RESEND v5 16/21] dt-bindings: mtd: Deprecate the nand-ecc-mode property
From: Boris Brezillon @ 2020-05-26 23:12 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200526195633.11543-17-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:28 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> This property does not describe very well its purpose: it describes
> the ECC engine type, also called provider. Deprecate it in favor of
> nand-ecc-provider. In the mean time, update a bit the description of
> the property to make it more accurate.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> .../devicetree/bindings/mtd/nand-controller.yaml | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> index a35ff8227427..256080ba50bd 100644
> --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> @@ -46,16 +46,17 @@ patternProperties:
> description:
> Contains the native Ready/Busy IDs.
>
> - nand-ecc-mode:
> + nand-ecc-provider:
> allOf:
> - $ref: /schemas/types.yaml#/definitions/string
> - enum: [ none, soft, hw, on-die ]
^"on-controller" ?
> description:
> - Desired ECC engine, either hardware (most of the time
> - embedded in the NAND controller) or software correction
> - (Linux will handle the calculations). soft_bch is deprecated
> - and should be replaced by soft and nand-ecc-algo.
> - hw_syndrome is deprecated and should be
> + Desired ECC engine provider, either hardware (most of the time
^Preferred ECC engine type, ...
> + embedded in the NAND controller, but can also be external) or
> + software correction (the OS will handle the calculations).
> + The nand-ecc-mode property is deprecated in favor of this one.
> + soft_bch is deprecated and should be replaced by soft and
> + nand-ecc-algo. hw_syndrome is deprecated and should be
> replaced by hw and nand-ecc-placement.
Which you do here, so patch 15 is not needed.
>
> nand-ecc-placement:
> @@ -148,7 +149,7 @@ examples:
>
> nand@0 {
> reg = <0>;
> - nand-ecc-mode = "soft";
> + nand-ecc-provider = "soft";
> nand-ecc-algo = "bch";
>
> /* controller specific properties */
^ permalink raw reply
* Re: [RESEND v5 14/21] dt-bindings: mtd: Add the nand-ecc-placement property
From: Rob Herring @ 2020-05-26 23:10 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Rob Herring, Mark Rutland, linux-mtd,
Tudor Ambarus, devicetree, Boris Brezillon, Vignesh Raghavendra
In-Reply-To: <20200526195633.11543-15-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:26 +0200, Miquel Raynal wrote:
> Placement is where the ECC bytes are expected.
> No value means the controller can do whatever it desires (default is
> to put ECC bytes at the end of the OOB area). Alternative placement is
> "interleaved" (also sometimes referred as "syndrome") where data and
> OOB are mixed.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> .../devicetree/bindings/mtd/nand-controller.yaml | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
Documentation/devicetree/bindings/mtd/nand-controller.yaml: while scanning a plain scalar
in "<unicode string>", line 64, column 11
found a tab character that violates indentation
in "<unicode string>", line 65, column 1
Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/mtd/nand-controller.example.dts' failed
make[1]: *** [Documentation/devicetree/bindings/mtd/nand-controller.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/nand-controller.yaml: ignoring, error parsing file
warning: no schema found in file: ./Documentation/devicetree/bindings/mtd/nand-controller.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/nand-controller.yaml: ignoring, error parsing file
warning: no schema found in file: ./Documentation/devicetree/bindings/mtd/nand-controller.yaml
Makefile:1300: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1298408
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
^ permalink raw reply
* Re: [RESEND v5 15/21] dt-bindings: mtd: Deprecate hw_syndrome from the ECC modes
From: Boris Brezillon @ 2020-05-26 23:09 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200526195633.11543-16-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:27 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> This mode should not be used anymore, it is actually a mix between a
> mode and a placement.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> Documentation/devicetree/bindings/mtd/nand-controller.yaml | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> index 35512f2c66fa..a35ff8227427 100644
> --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> @@ -49,12 +49,14 @@ patternProperties:
> nand-ecc-mode:
> allOf:
> - $ref: /schemas/types.yaml#/definitions/string
> - - enum: [ none, soft, hw, hw_syndrome, on-die ]
> + - enum: [ none, soft, hw, on-die ]
> description:
> Desired ECC engine, either hardware (most of the time
> embedded in the NAND controller) or software correction
> (Linux will handle the calculations). soft_bch is deprecated
> and should be replaced by soft and nand-ecc-algo.
> + hw_syndrome is deprecated and should be
> + replaced by hw and nand-ecc-placement.
Well, I'd expect the whole property to be deprecated in favor of
nand-ecc-engine-type.
>
> nand-ecc-placement:
> allOf:
^ permalink raw reply
* Re: [RESEND v5 14/21] dt-bindings: mtd: Add the nand-ecc-placement property
From: Boris Brezillon @ 2020-05-26 23:08 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200526195633.11543-15-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:26 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Placement is where the ECC bytes are expected.
> No value means the controller can do whatever it desires (default is
> to put ECC bytes at the end of the OOB area). Alternative placement is
> "interleaved" (also sometimes referred as "syndrome") where data and
> OOB are mixed.
Hm, unless you can figure out what all controllers use, I'd suggest to
have the default mean "undefined", as in "controller put the ECC bytes
somewhere, but we don't where that is". Then let the explicit values be
"oob" or "interleaved".
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> .../devicetree/bindings/mtd/nand-controller.yaml | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> index d529f8587ba6..35512f2c66fa 100644
> --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> @@ -56,6 +56,16 @@ patternProperties:
> (Linux will handle the calculations). soft_bch is deprecated
> and should be replaced by soft and nand-ecc-algo.
>
> + nand-ecc-placement:
> + allOf:
> + - $ref: /schemas/types.yaml#/definitions/string
> + - enum: [ free, interleaved ]
> + description:
> + Location for the ECC bytes. Free is the default and means the
> + controller in charge is free to put them where it wants.
> + Default state is to put ECC bytes at the end of the OOB area.
> + Otherwise, ECC bytes may be interleaved with data.
> +
> nand-ecc-algo:
> allOf:
> - $ref: /schemas/types.yaml#/definitions/string
^ permalink raw reply
* Re: [PATCH V5 2/8] dt-bindings: clock: add ipq6018 a53 pll compatible
From: Rob Herring @ 2020-05-26 23:08 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: agross, bjorn.andersson, mturquette, sboyd, jassisinghbrar,
linux-arm-msm, linux-clk, devicetree, linux-kernel
In-Reply-To: <1590314686-11749-3-git-send-email-sivaprak@codeaurora.org>
On Sun, May 24, 2020 at 03:34:40PM +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>
> ---
> [V5]
> * Addressed Bjorn's review comment.
> now the a53 dt-binding represents ipq a53 pll as well.
> .../devicetree/bindings/clock/qcom,a53pll.yaml | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 12/13] dt-bindings: clock: Add Marvell MMP Audio Clock Controller binding
From: Rob Herring @ 2020-05-26 23:06 UTC (permalink / raw)
To: Lubomir Rintel
Cc: Rob Herring, linux-arm-kernel, devicetree, Stephen Boyd,
Michael Turquette, linux-clk, linux-kernel
In-Reply-To: <20200519224151.2074597-13-lkundrak@v3.sk>
On Wed, 20 May 2020 00:41:50 +0200, Lubomir Rintel wrote:
> This describes the bindings for a controller that generates master and bit
> clocks for the I2S interface.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
>
> ---
> Changes since v1:
> - Fix commit message wording
> - Define MMP2_CLK_AUDIO_NR_CLKS
> - Make clock ids start at 0, not 1
> - Fix dt-bindings/clock/marvell,mmp2-audio.h file name
> - Rename node from "clocks" to "clock-controller"
>
> .../clock/marvell,mmp2-audio-clock.yaml | 74 +++++++++++++++++++
> .../dt-bindings/clock/marvell,mmp2-audio.h | 10 +++
> 2 files changed, 84 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml
> create mode 100644 include/dt-bindings/clock/marvell,mmp2-audio.h
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 09/13] dt-bindings: clock: Make marvell,mmp2-clock a power controller
From: Rob Herring @ 2020-05-26 23:05 UTC (permalink / raw)
To: Lubomir Rintel
Cc: Stephen Boyd, Michael Turquette, linux-clk, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <20200519224151.2074597-10-lkundrak@v3.sk>
On Wed, May 20, 2020 at 12:41:47AM +0200, Lubomir Rintel wrote:
> This is a binding for the MMP2 power management units. As such apart from
> providing the clocks, they also manage the power islands.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
>
> ---
> Changes since v2:
> - Added this patch
>
> .../devicetree/bindings/clock/marvell,mmp2-clock.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [RESEND v5 12/21] mtd: rawnand: Deprecate nand-ecc-mode in favor of nand-ecc-provider
From: Boris Brezillon @ 2020-05-26 23:03 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200526195633.11543-13-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:24 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Use nand-ecc-provider as the provider DT property. Fallback to
> nand-ecc-mode if the property does not exist.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 69c1b7ab938e..7d17d52cdd34 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5036,7 +5036,9 @@ of_get_nand_ecc_engine_type(struct device_node *np)
> const char *pm;
> int err;
>
> - err = of_property_read_string(np, "nand-ecc-mode", &pm);
> + err = of_property_read_string(np, "nand-ecc-provider", &pm);
"nand-ecc-engine-type"
And I'd prefer to have a different string table for that one, so we get
rid of the confusing "hw" type (which is actually "on-controller").
> + if (err)
> + err = of_property_read_string(np, "nand-ecc-mode", &pm);
> if (err)
> return NAND_ECC_ENGINE_INVALID;
>
^ permalink raw reply
* Re: [RESEND v5 09/21] mtd: rawnand: Create a new enumeration to describe properly ECC types
From: Boris Brezillon @ 2020-05-26 22:59 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20200526195633.11543-10-miquel.raynal@bootlin.com>
On Tue, 26 May 2020 21:56:21 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Now that the misleading mix between ECC engine type and OOB placement
> has been addressed, add a new enumeration to properly define ECC types
> (also called provider or mode).
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 7 +++++++
> include/linux/mtd/rawnand.h | 16 ++++++++++++++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 515cd4681660..5c6ab5b93270 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5018,6 +5018,13 @@ static const char * const nand_ecc_modes[] = {
> [NAND_ECC_ON_DIE] = "on-die",
> };
>
> +static const char * const nand_ecc_engine_providers[] = {
This table is not used here, are you sure it should be introduced now?
> + [NAND_ECC_ENGINE_NONE] = "none",
> + [NAND_ECC_ENGINE_SOFT] = "soft",
> + [NAND_ECC_ENGINE_CONTROLLER] = "hw",
^ "on-controller" ?
> + [NAND_ECC_ENGINE_ON_DIE] = "on-die",
> +};
> +
> static const char * const nand_ecc_placement[] = {
> [NAND_ECC_PLACEMENT_INTERLEAVED] = "interleaved",
> };
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index dc909fb977c7..a2078c5f3d21 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -92,6 +92,22 @@ enum nand_ecc_mode {
> NAND_ECC_ON_DIE,
> };
>
> +/**
> + * enum nand_ecc_engine_type - NAND ECC engine type/provider
> + * @NAND_ECC_ENGINE_INVALID: Invalid value
> + * @NAND_ECC_ENGINE_NONE: No ECC correction
> + * @NAND_ECC_ENGINE_SOFT: Software ECC correction
> + * @NAND_ECC_ENGINE_CONTROLLER: Hardware controller ECC correction
> + * @NAND_ECC_ENGINE_ON_DIE: On chip hardware ECC correction
> + */
> +enum nand_ecc_engine_type {
> + NAND_ECC_ENGINE_INVALID,
> + NAND_ECC_ENGINE_NONE,
> + NAND_ECC_ENGINE_SOFT,
> + NAND_ECC_ENGINE_CONTROLLER,
> + NAND_ECC_ENGINE_ON_DIE,
> +};
> +
> /**
> * enum nand_ecc_placement - NAND ECC placement
> * @NAND_ECC_PLACEMENT_FREE: The driver can decide where to put ECC bytes.
^ 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