Re: [PATCH 8/8] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC

2020-03-06 Thread Geert Uytterhoeven
Hi Alex,

On Thu, Mar 5, 2020 at 3:36 PM Alex Riesen  wrote:
> Geert Uytterhoeven, Mon, Mar 02, 2020 17:13:30 +0100:
> > On Mon, Mar 2, 2020 at 5:09 PM Alex Riesen  
> > wrote:
> > > Geert Uytterhoeven, Mon, Mar 02, 2020 16:32:32 +0100:
> > > >
> > > > The #clock-cells should be in the main video-receiver node.
> > > > Probably there is more than one clock output, so #clock-cells may be 1?
> > >
> > > AFAICS, the device can provide only this one clock line (audio master 
> > > clock
> > > for I2S output)... I shall re-check, just in case.
>
> And you're right, of course: the audio output formatting module of the ADV748x
> devices provides a set of clock lines related to the I2S pins: the already
> discussed master clock, left-right channel clock and the serial clock (bit
> clock?).
>
> > > > There is no need for a fixed-clock compatible, nor for clock-frequency
> > > > and clock-output-names.
> > > >
> > > > But most important: this should be documented in the adv748x DT 
> > > > bindings,
> > > > and implemented in the adv748x driver.
> > >
> > > So if the driver is to export that clock for the kernel (like in this 
> > > case),
> > > it must implement its support?
> >
> > Exactly.  Unless that pin is hardcoded to output a fixed clock, in which 
> > case
> > you can just override the existing audio_clk_c rate.
>
> Just to try it out (I'll set #clock-cells to 1), I registered a fixed rate
> clock in the driver, added a clock provider:
>
> adv748x_probe:
>
> clk = clk_register_fixed_rate(state->dev,
>   "clk-hdmi-i2s-mclk",
>   NULL /* parent_name */,
>   0/* flags */,
>   12288000 /* rate */);
> of_clk_add_provider(state->dev->of_node, of_clk_src_simple_get, clk);
>
> And removed the audio_clk_c frequency setting. I also replaced the audio_clk_c
> in the list of input clocks of the R-Car-side sound card with the phandle of
> the adv7482 main node:
>
> salvator-common.dtsi:
>
> &i2c4 {
> status = "okay";
>
> adv7482_hdmi_decoder: video-receiver@70 {
> #clock-cells = <0>; // to be replaced with <1>
> };
> };
>
> &rcar_sound {
> clocks = ..., <&adv7482_hdmi_decoder>, ...;
> };
>
> As everything continues to work as before, I assume that at least the clock
> dependencies were resolved.
>
> Is there a way to verify that the added input clock is actually used?
> IOW, if its frequency is actually has been programmed into the ssi4 (R-Car
> receiving hardware) registers, and not just a left-over from previuos attempts
> or plain default setting?

Have a look at /sys/kernel/debug/clk/clk_summary

> As the ADV748x devices seem to provide also the clocks for video outputs, will
> it make any sense to place the clock definition into the port node?
> Or should all provided clocks be indexed in the main device node?

Sorry, I don't know.

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 8/8] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC

2020-03-06 Thread Laurent Pinchart
Hi Alex,

On Thu, Mar 05, 2020 at 03:36:28PM +0100, Alex Riesen wrote:
> Geert Uytterhoeven, Mon, Mar 02, 2020 17:13:30 +0100:
> > On Mon, Mar 2, 2020 at 5:09 PM Alex Riesen  
> > wrote:
> > > Geert Uytterhoeven, Mon, Mar 02, 2020 16:32:32 +0100:
> > > >
> > > > The #clock-cells should be in the main video-receiver node.
> > > > Probably there is more than one clock output, so #clock-cells may be 1?
> > >
> > > AFAICS, the device can provide only this one clock line (audio master 
> > > clock
> > > for I2S output)... I shall re-check, just in case.
> 
> And you're right, of course: the audio output formatting module of the ADV748x
> devices provides a set of clock lines related to the I2S pins: the already
> discussed master clock, left-right channel clock and the serial clock (bit
> clock?).

I don't think we need to model the last two clocks through CCF though,
they're part of the I2S protocol, not clock sources that need to be
explicitly controlled (or queried).

> > > > There is no need for a fixed-clock compatible, nor for clock-frequency
> > > > and clock-output-names.
> > > >
> > > > But most important: this should be documented in the adv748x DT 
> > > > bindings,
> > > > and implemented in the adv748x driver.
> > >
> > > So if the driver is to export that clock for the kernel (like in this 
> > > case),
> > > it must implement its support?
> > 
> > Exactly.  Unless that pin is hardcoded to output a fixed clock, in which 
> > case
> > you can just override the existing audio_clk_c rate.
> 
> Just to try it out (I'll set #clock-cells to 1), I registered a fixed rate
> clock in the driver, added a clock provider:
> 
> adv748x_probe:
> 
> clk = clk_register_fixed_rate(state->dev,
> "clk-hdmi-i2s-mclk",
> NULL /* parent_name */,
> 0/* flags */,
> 12288000 /* rate */);
> of_clk_add_provider(state->dev->of_node, of_clk_src_simple_get, clk);
> 
> And removed the audio_clk_c frequency setting. I also replaced the audio_clk_c
> in the list of input clocks of the R-Car-side sound card with the phandle of
> the adv7482 main node:
> 
> salvator-common.dtsi:
> 
> &i2c4 {
>   status = "okay";
> 
>   adv7482_hdmi_decoder: video-receiver@70 {
>   #clock-cells = <0>; // to be replaced with <1>
>   };
> };
> 
> &rcar_sound {
>   clocks = ..., <&adv7482_hdmi_decoder>, ...;
> };
> 
> As everything continues to work as before, I assume that at least the clock
> dependencies were resolved.

This looks good to me.

> Is there a way to verify that the added input clock is actually used?
> IOW, if its frequency is actually has been programmed into the ssi4 (R-Car
> receiving hardware) registers, and not just a left-over from previuos attempts
> or plain default setting?
> 
> As the ADV748x devices seem to provide also the clocks for video outputs, will
> it make any sense to place the clock definition into the port node?
> Or should all provided clocks be indexed in the main device node?

Those clocks are part of the CSI-2 protocol and also don't need to be
explicitly controlled. As far as I can tell from a quick check of the
ADV7482 documentation, only the I2S MCLK is a general-purpose clock that
needs to be exposed.

-- 
Regards,

Laurent Pinchart
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 8/8] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC

2020-03-06 Thread Alex Riesen
Hi Laurent,

Laurent Pinchart, Fri, Mar 06, 2020 14:16:32 +0100:
> On Thu, Mar 05, 2020 at 03:36:28PM +0100, Alex Riesen wrote:
> > Geert Uytterhoeven, Mon, Mar 02, 2020 17:13:30 +0100:
> > > On Mon, Mar 2, 2020 at 5:09 PM Alex Riesen  
> > > wrote:
> > > > Geert Uytterhoeven, Mon, Mar 02, 2020 16:32:32 +0100:
> > > > >
> > > > > The #clock-cells should be in the main video-receiver node.
> > > > > Probably there is more than one clock output, so #clock-cells may be 
> > > > > 1?
> > > >
> > > > AFAICS, the device can provide only this one clock line (audio master 
> > > > clock
> > > > for I2S output)... I shall re-check, just in case.
> > 
> > And you're right, of course: the audio output formatting module of the 
> > ADV748x
> > devices provides a set of clock lines related to the I2S pins: the already
> > discussed master clock, left-right channel clock and the serial clock (bit
> > clock?).
> 
> I don't think we need to model the last two clocks through CCF though,
> they're part of the I2S protocol, not clock sources that need to be
> explicitly controlled (or queried).

That's good, because I'm right now having hard time finding out how to
calculate the frequencies!

> > Just to try it out (I'll set #clock-cells to 1), I registered a fixed rate
> > clock in the driver, added a clock provider:
> > 
> > adv748x_probe:
> > 
> > clk = clk_register_fixed_rate(state->dev,
> >   "clk-hdmi-i2s-mclk",
> >   NULL /* parent_name */,
> >   0/* flags */,
> >   12288000 /* rate */);
> > of_clk_add_provider(state->dev->of_node, of_clk_src_simple_get, clk);
> > 
> > And removed the audio_clk_c frequency setting. I also replaced the 
> > audio_clk_c
> > in the list of input clocks of the R-Car-side sound card with the phandle of
> > the adv7482 main node:
> > 
> > salvator-common.dtsi:
> > 
> > &i2c4 {
> > status = "okay";
> > 
> > adv7482_hdmi_decoder: video-receiver@70 {
> > #clock-cells = <0>; // to be replaced with <1>
> > };
> > };
> > 
> > &rcar_sound {
> > clocks = ..., <&adv7482_hdmi_decoder>, ...;
> > };
> > 
> > As everything continues to work as before, I assume that at least the clock
> > dependencies were resolved.
> 
> This looks good to me.

Ok, I settle on this than.

> > Is there a way to verify that the added input clock is actually used?
> > IOW, if its frequency is actually has been programmed into the ssi4 (R-Car
> > receiving hardware) registers, and not just a left-over from previuos 
> > attempts
> > or plain default setting?
> > 
> > As the ADV748x devices seem to provide also the clocks for video outputs, 
> > will
> > it make any sense to place the clock definition into the port node?
> > Or should all provided clocks be indexed in the main device node?
> 
> Those clocks are part of the CSI-2 protocol and also don't need to be
> explicitly controlled. As far as I can tell from a quick check of the
> ADV7482 documentation, only the I2S MCLK is a general-purpose clock that
> needs to be exposed.

Thanks, that's good to know!

Do you know, by chance, which of the snd_soc* callbacks should be used to
implement setting of the MCLK? The one in snd_soc_component_driver or
snd_soc_dai_driver->ops (snd_soc_dai_ops)?

Or how the userspace interface looks like? Or, if there is no userspace
interface for this, how the MCLK is supposed to be set? Through mclk-fs?

Regards,
Alex

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 8/8] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC

2020-03-06 Thread Laurent Pinchart
Hi Alex,

(CC'ing Morimoto-san)

On Fri, Mar 06, 2020 at 02:41:54PM +0100, Alex Riesen wrote:
> Laurent Pinchart, Fri, Mar 06, 2020 14:16:32 +0100:
> > On Thu, Mar 05, 2020 at 03:36:28PM +0100, Alex Riesen wrote:
> >> Geert Uytterhoeven, Mon, Mar 02, 2020 17:13:30 +0100:
> >>> On Mon, Mar 2, 2020 at 5:09 PM Alex Riesen  
> >>> wrote:
>  Geert Uytterhoeven, Mon, Mar 02, 2020 16:32:32 +0100:
> >
> > The #clock-cells should be in the main video-receiver node.
> > Probably there is more than one clock output, so #clock-cells may be 1?
> 
>  AFAICS, the device can provide only this one clock line (audio master 
>  clock
>  for I2S output)... I shall re-check, just in case.
> >> 
> >> And you're right, of course: the audio output formatting module of the 
> >> ADV748x
> >> devices provides a set of clock lines related to the I2S pins: the already
> >> discussed master clock, left-right channel clock and the serial clock (bit
> >> clock?).
> > 
> > I don't think we need to model the last two clocks through CCF though,
> > they're part of the I2S protocol, not clock sources that need to be
> > explicitly controlled (or queried).
> 
> That's good, because I'm right now having hard time finding out how to
> calculate the frequencies!
> 
> >> Just to try it out (I'll set #clock-cells to 1), I registered a fixed rate
> >> clock in the driver, added a clock provider:
> >> 
> >> adv748x_probe:
> >> 
> >> clk = clk_register_fixed_rate(state->dev,
> >>  "clk-hdmi-i2s-mclk",
> >>  NULL /* parent_name */,
> >>  0/* flags */,
> >>  12288000 /* rate */);
> >> of_clk_add_provider(state->dev->of_node, of_clk_src_simple_get, clk);
> >> 
> >> And removed the audio_clk_c frequency setting. I also replaced the 
> >> audio_clk_c
> >> in the list of input clocks of the R-Car-side sound card with the phandle 
> >> of
> >> the adv7482 main node:
> >> 
> >> salvator-common.dtsi:
> >> 
> >> &i2c4 {
> >>status = "okay";
> >> 
> >>adv7482_hdmi_decoder: video-receiver@70 {
> >>#clock-cells = <0>; // to be replaced with <1>
> >>};
> >> };
> >> 
> >> &rcar_sound {
> >>clocks = ..., <&adv7482_hdmi_decoder>, ...;
> >> };
> >> 
> >> As everything continues to work as before, I assume that at least the clock
> >> dependencies were resolved.
> > 
> > This looks good to me.
> 
> Ok, I settle on this than.
> 
> >> Is there a way to verify that the added input clock is actually used?
> >> IOW, if its frequency is actually has been programmed into the ssi4 (R-Car
> >> receiving hardware) registers, and not just a left-over from previuos 
> >> attempts
> >> or plain default setting?
> >> 
> >> As the ADV748x devices seem to provide also the clocks for video outputs, 
> >> will
> >> it make any sense to place the clock definition into the port node?
> >> Or should all provided clocks be indexed in the main device node?
> > 
> > Those clocks are part of the CSI-2 protocol and also don't need to be
> > explicitly controlled. As far as I can tell from a quick check of the
> > ADV7482 documentation, only the I2S MCLK is a general-purpose clock that
> > needs to be exposed.
> 
> Thanks, that's good to know!
> 
> Do you know, by chance, which of the snd_soc* callbacks should be used to
> implement setting of the MCLK? The one in snd_soc_component_driver or
> snd_soc_dai_driver->ops (snd_soc_dai_ops)?
> 
> Or how the userspace interface looks like? Or, if there is no userspace
> interface for this, how the MCLK is supposed to be set? Through mclk-fs?

I'm afraid my knowledge of the sound subsystem is limited. Morimoto-san
is the main developer and maintainer of Renesas sound drivers.
Morimoto-sensei, would you have an answer to that question ? :-)

-- 
Regards,

Laurent Pinchart
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 2/2] staging: wilc1000: use single DT binding documentation for SDIO and SPI

2020-03-06 Thread Ajay.Kathat
From: Ajay Singh 

Merged the DT binding documentation of SDIO and SPI into a single file.
Removed documentation for some of the properties which are not required
and handled review comments received in [1] & [2].

[1]. https://lore.kernel.org/linux-wireless/20200303020230.GA15543@bogus
[2]. https://lore.kernel.org/linux-wireless/20200303015558.GA6876@bogus

Signed-off-by: Ajay Singh 
---
 .../wilc1000/microchip,wilc1000,sdio.yaml | 68 ---
 ...c1000,spi.yaml => microchip,wilc1000.yaml} | 44 
 drivers/staging/wilc1000/sdio.c   |  4 +-
 drivers/staging/wilc1000/spi.c|  2 +-
 4 files changed, 34 insertions(+), 84 deletions(-)
 delete mode 100644 drivers/staging/wilc1000/microchip,wilc1000,sdio.yaml
 rename drivers/staging/wilc1000/{microchip,wilc1000,spi.yaml => 
microchip,wilc1000.yaml} (53%)

diff --git a/drivers/staging/wilc1000/microchip,wilc1000,sdio.yaml 
b/drivers/staging/wilc1000/microchip,wilc1000,sdio.yaml
deleted file mode 100644
index 9df7327bc668..
--- a/drivers/staging/wilc1000/microchip,wilc1000,sdio.yaml
+++ /dev/null
@@ -1,68 +0,0 @@
-# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
-%YAML 1.2

-$id: http://devicetree.org/schemas/net/wireless/microchip,wilc1000,sdio.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Microchip WILC wireless SDIO devicetree bindings
-
-maintainers:
-  - Adham Abozaeid 
-  - Ajay Singh 
-
-description:
-  The wilc1000 chips can be connected via SDIO. The node is used to
-  specify child node to the SDIO controller that connects the device
-  to the system.
-
-properties:
-  compatible:
-const: microchip,wilc1000-sdio
-
-  interrupts:
-maxItems: 1
-
-  reg:
-description: Slot ID used in the controller.
-maxItems: 1
-
-  clocks:
-description: phandle to the clock connected on rtc clock line.
-maxItems: 1
-
-  bus-width:
-description: The number of data lines wired up the slot.
-allOf:
-  - $ref: /schemas/types.yaml#/definitions/uint32
-  - enum: [1, 4, 8]
-  - default: 1
-
-required:
-  - compatible
-  - interrupts
-  - reg
-
-examples:
-  - |
-mmc1: mmc@fc00 {
-  #address-cells = <1>;
-  #size-cells = <0>;
-  pinctrl-names = "default";
-  pinctrl-0 = <&pinctrl_mmc1_clk_cmd_dat0 &pinctrl_mmc1_dat1_3>;
-  non-removable;
-  vmmc-supply = <&vcc_mmc1_reg>;
-  vqmmc-supply = <&vcc_3v3_reg>;
-  status = "okay";
-  wilc_sdio@0 {
-compatible = "microchip,wilc1000-sdio";
-interrupt-parent = <&pioC>;
-interrupts = <27 0>;
-  reg = <0>;
-  clocks = <&pck1>;
-  clock-names = "rtc_clk";
-  assigned-clocks = <&pck1>;
-  assigned-clock-rates = <32768>;
-  status = "okay";
-  bus-width = <4>;
-};
-};
diff --git a/drivers/staging/wilc1000/microchip,wilc1000,spi.yaml 
b/drivers/staging/wilc1000/microchip,wilc1000.yaml
similarity index 53%
rename from drivers/staging/wilc1000/microchip,wilc1000,spi.yaml
rename to drivers/staging/wilc1000/microchip,wilc1000.yaml
index dd5e8da1f562..a1914449ad07 100644
--- a/drivers/staging/wilc1000/microchip,wilc1000,spi.yaml
+++ b/drivers/staging/wilc1000/microchip,wilc1000.yaml
@@ -1,22 +1,22 @@
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/net/wireless/microchip,wilc1000,spi.yaml#
+$id: http://devicetree.org/schemas/net/wireless/microchip,wilc1000.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Microchip WILC wireless SPI devicetree bindings
+title: Microchip WILC wireless devicetree bindings
 
 maintainers:
   - Adham Abozaeid 
   - Ajay Singh 
 
 description:
-  The wilc1000 chips can be connected via SPI. This document describes
-  the binding for the SPI connected module.
+  The wilc1000 chips can be connected via SPI or SDIO. This document
+  describes the binding to connect wilc devices.
 
 properties:
   compatible:
-const: microchip,wilc1000-spi
+const: microchip,wilc1000
 
   spi-max-frequency:
 description: Maximum SPI clocking speed of device in Hz.
@@ -33,9 +33,11 @@ properties:
 description: phandle to the clock connected on rtc clock line.
 maxItems: 1
 
+  clock-names:
+const: rtc
+
 required:
   - compatible
-  - spi-max-frequency
   - reg
   - interrupts
 
@@ -45,17 +47,33 @@ examples:
   #address-cells = <1>;
   #size-cells = <0>;
   cs-gpios = <&pioB 21 0>;
-  status = "okay";
-  wilc_spi@0 {
-compatible = "microchip,wilc1000-spi";
+  wifi@0 {
+compatible = "microchip,wilc1000";
 spi-max-frequency = <4800>;
 reg = <0>;
 interrupt-parent = <&pioC>;
 interrupts = <27 0>;
 clocks = <&pck1>;
-clock-names = "rtc_clk";
-assigned-clocks = <&pck1>;
-assigned-clock-rates = <32768>;
-status = "okay";
+clock-names = "rtc";
+  

[PATCH 1/2] staging: wilc1000: use 'interrupts' property instead of 'irq-gpio'

2020-03-06 Thread Ajay.Kathat
From: Ajay Singh 

Make use of 'interrupts' property instead of using gpio for handling
the interrupt as suggested in [1].

[1]. https://lore.kernel.org/linux-wireless/20200303015558.GA6876@bogus/

Signed-off-by: Ajay Singh 
---
 .../net/wireless/microchip,wilc1000.yaml  | 79 +++
 .../wilc1000/microchip,wilc1000,sdio.yaml |  8 +-
 .../wilc1000/microchip,wilc1000,spi.yaml  |  8 +-
 drivers/staging/wilc1000/netdev.c | 24 ++
 drivers/staging/wilc1000/netdev.h |  1 -
 drivers/staging/wilc1000/sdio.c   | 31 +++-
 drivers/staging/wilc1000/spi.c| 15 +---
 drivers/staging/wilc1000/wlan.h   |  1 -
 8 files changed, 108 insertions(+), 59 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml

diff --git 
a/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml 
b/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml
new file mode 100644
index ..a1914449ad07
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/wireless/microchip,wilc1000.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip WILC wireless devicetree bindings
+
+maintainers:
+  - Adham Abozaeid 
+  - Ajay Singh 
+
+description:
+  The wilc1000 chips can be connected via SPI or SDIO. This document
+  describes the binding to connect wilc devices.
+
+properties:
+  compatible:
+const: microchip,wilc1000
+
+  spi-max-frequency:
+description: Maximum SPI clocking speed of device in Hz.
+maxItems: 1
+
+  reg:
+description: Chip select address of device.
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+
+  clocks:
+description: phandle to the clock connected on rtc clock line.
+maxItems: 1
+
+  clock-names:
+const: rtc
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+examples:
+  - |
+spi1: spi@fc018000 {
+  #address-cells = <1>;
+  #size-cells = <0>;
+  cs-gpios = <&pioB 21 0>;
+  wifi@0 {
+compatible = "microchip,wilc1000";
+spi-max-frequency = <4800>;
+reg = <0>;
+interrupt-parent = <&pioC>;
+interrupts = <27 0>;
+clocks = <&pck1>;
+clock-names = "rtc";
+  };
+};
+
+  - |
+mmc1: mmc@fc00 {
+  #address-cells = <1>;
+  #size-cells = <0>;
+  pinctrl-names = "default";
+  pinctrl-0 = <&pinctrl_mmc1_clk_cmd_dat0 &pinctrl_mmc1_dat1_3>;
+  non-removable;
+  vmmc-supply = <&vcc_mmc1_reg>;
+  vqmmc-supply = <&vcc_3v3_reg>;
+  bus-width = <4>;
+  wifi@0 {
+compatible = "microchip,wilc1000";
+reg = <0>;
+interrupt-parent = <&pioC>;
+interrupts = <27 0>;
+clocks = <&pck1>;
+clock-names = "rtc";
+  };
+};
diff --git a/drivers/staging/wilc1000/microchip,wilc1000,sdio.yaml 
b/drivers/staging/wilc1000/microchip,wilc1000,sdio.yaml
index b338f569f7e2..9df7327bc668 100644
--- a/drivers/staging/wilc1000/microchip,wilc1000,sdio.yaml
+++ b/drivers/staging/wilc1000/microchip,wilc1000,sdio.yaml
@@ -19,8 +19,7 @@ properties:
   compatible:
 const: microchip,wilc1000-sdio
 
-  irq-gpios:
-description: The GPIO phandle connect to a host IRQ.
+  interrupts:
 maxItems: 1
 
   reg:
@@ -40,7 +39,7 @@ properties:
 
 required:
   - compatible
-  - irq-gpios
+  - interrupts
   - reg
 
 examples:
@@ -56,7 +55,8 @@ examples:
   status = "okay";
   wilc_sdio@0 {
 compatible = "microchip,wilc1000-sdio";
-  irq-gpios = <&pioC 27 0>;
+interrupt-parent = <&pioC>;
+interrupts = <27 0>;
   reg = <0>;
   clocks = <&pck1>;
   clock-names = "rtc_clk";
diff --git a/drivers/staging/wilc1000/microchip,wilc1000,spi.yaml 
b/drivers/staging/wilc1000/microchip,wilc1000,spi.yaml
index cc8ed64ce627..dd5e8da1f562 100644
--- a/drivers/staging/wilc1000/microchip,wilc1000,spi.yaml
+++ b/drivers/staging/wilc1000/microchip,wilc1000,spi.yaml
@@ -26,8 +26,7 @@ properties:
 description: Chip select address of device.
 maxItems: 1
 
-  irq-gpios:
-description: The GPIO phandle connect to a host IRQ.
+  interrupts:
 maxItems: 1
 
   clocks:
@@ -38,7 +37,7 @@ required:
   - compatible
   - spi-max-frequency
   - reg
-  - irq-gpios
+  - interrupts
 
 examples:
   - |
@@ -51,7 +50,8 @@ examples:
 compatible = "microchip,wilc1000-spi";
 spi-max-frequency = <4800>;
 reg = <0>;
-irq-gpios = <&pioC 27 0>;
+interrupt-parent = <&pioC>;
+interrupts = <27 0>;
 clocks = <&pck1>;
 clock-names = "rtc_clk";
 assigned-clocks = <&pck1>;
diff --git a/drivers/staging/wilc1000/netdev.c 
b/drivers/staging/wilc1000/netdev.c
index 045f5cdfdca0..a61c1a7aefa8 100644
--- a

[PATCH 0/2] staging: wilc1000: handle DT binding documentation comments

2020-03-06 Thread Ajay.Kathat
From: Ajay Singh 

This patch series contains changes to handle DT binding documentation
related review comments. The changes were suggested during the full
driver review [1] & [2].
First submitting these patches to staging and later will include them
as part of the full driver review patch series.

[1]. https://patchwork.kernel.org/patch/11415897
[2]. https://patchwork.kernel.org/patch/11415901

Ajay Singh (2):
  staging: wilc1000: use 'interrupts' property instead of 'irq-gpio'
  staging: wilc1000: use single DT binding documentation for SDIO and
SPI

 .../net/wireless/microchip,wilc1000.yaml  | 79 +++
 .../wilc1000/microchip,wilc1000,sdio.yaml | 68 
 .../wilc1000/microchip,wilc1000,spi.yaml  | 61 --
 .../staging/wilc1000/microchip,wilc1000.yaml  | 79 +++
 drivers/staging/wilc1000/netdev.c | 24 ++
 drivers/staging/wilc1000/netdev.h |  1 -
 drivers/staging/wilc1000/sdio.c   | 35 
 drivers/staging/wilc1000/spi.c| 17 +---
 drivers/staging/wilc1000/wlan.h   |  1 -
 9 files changed, 182 insertions(+), 183 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml
 delete mode 100644 drivers/staging/wilc1000/microchip,wilc1000,sdio.yaml
 delete mode 100644 drivers/staging/wilc1000/microchip,wilc1000,spi.yaml
 create mode 100644 drivers/staging/wilc1000/microchip,wilc1000.yaml

-- 
2.24.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/2] staging: wilc1000: use 'interrupts' property instead of 'irq-gpio'

2020-03-06 Thread Rob Herring
On Fri, Mar 6, 2020 at 8:44 AM  wrote:
>
> From: Ajay Singh 
>
> Make use of 'interrupts' property instead of using gpio for handling
> the interrupt as suggested in [1].
>
> [1]. https://lore.kernel.org/linux-wireless/20200303015558.GA6876@bogus/
>
> Signed-off-by: Ajay Singh 
> ---
>  .../net/wireless/microchip,wilc1000.yaml  | 79 +++
>  .../wilc1000/microchip,wilc1000,sdio.yaml |  8 +-
>  .../wilc1000/microchip,wilc1000,spi.yaml  |  8 +-

Bindings should be a separate patch.

>  drivers/staging/wilc1000/netdev.c | 24 ++
>  drivers/staging/wilc1000/netdev.h |  1 -
>  drivers/staging/wilc1000/sdio.c   | 31 +++-
>  drivers/staging/wilc1000/spi.c| 15 +---
>  drivers/staging/wilc1000/wlan.h   |  1 -
>  8 files changed, 108 insertions(+), 59 deletions(-)
>  create mode 100644 
> Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml
>
> diff --git 
> a/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml 
> b/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml
> new file mode 100644
> index ..a1914449ad07
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml
> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/wireless/microchip,wilc1000.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip WILC wireless devicetree bindings
> +
> +maintainers:
> +  - Adham Abozaeid 
> +  - Ajay Singh 
> +
> +description:
> +  The wilc1000 chips can be connected via SPI or SDIO. This document
> +  describes the binding to connect wilc devices.
> +
> +properties:
> +  compatible:
> +const: microchip,wilc1000
> +
> +  spi-max-frequency:
> +description: Maximum SPI clocking speed of device in Hz.
> +maxItems: 1

No need to redefine a common property. Just:

spi-max-frequency: true

> +
> +  reg:
> +description: Chip select address of device.

Drop this.

> +maxItems: 1
> +
> +  interrupts:
> +maxItems: 1
> +
> +  clocks:
> +description: phandle to the clock connected on rtc clock line.
> +maxItems: 1
> +
> +  clock-names:
> +const: rtc
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +examples:
> +  - |
> +spi1: spi@fc018000 {
> +  #address-cells = <1>;
> +  #size-cells = <0>;
> +  cs-gpios = <&pioB 21 0>;
> +  wifi@0 {
> +compatible = "microchip,wilc1000";
> +spi-max-frequency = <4800>;
> +reg = <0>;
> +interrupt-parent = <&pioC>;
> +interrupts = <27 0>;
> +clocks = <&pck1>;
> +clock-names = "rtc";
> +  };
> +};
> +
> +  - |
> +mmc1: mmc@fc00 {
> +  #address-cells = <1>;
> +  #size-cells = <0>;
> +  pinctrl-names = "default";
> +  pinctrl-0 = <&pinctrl_mmc1_clk_cmd_dat0 &pinctrl_mmc1_dat1_3>;
> +  non-removable;
> +  vmmc-supply = <&vcc_mmc1_reg>;
> +  vqmmc-supply = <&vcc_3v3_reg>;
> +  bus-width = <4>;
> +  wifi@0 {
> +compatible = "microchip,wilc1000";
> +reg = <0>;
> +interrupt-parent = <&pioC>;
> +interrupts = <27 0>;
> +clocks = <&pck1>;
> +clock-names = "rtc";
> +  };
> +};
> diff --git a/drivers/staging/wilc1000/microchip,wilc1000,sdio.yaml 
> b/drivers/staging/wilc1000/microchip,wilc1000,sdio.yaml
> index b338f569f7e2..9df7327bc668 100644
> --- a/drivers/staging/wilc1000/microchip,wilc1000,sdio.yaml
> +++ b/drivers/staging/wilc1000/microchip,wilc1000,sdio.yaml

Why aren't you just removing this file and the spi one?

Rob
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel