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

From: Rob Herring <robh@kernel.org>
To: Ajay.Kathat@microchip.com
Cc: linux-wireless <linux-wireless@vger.kernel.org>,
	"open list:STAGING SUBSYSTEM" <devel@driverdev.osuosl.org>,
	devicetree@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Adham.Abozaeid@microchip.com,
	Johannes Berg <johannes@sipsolutions.net>
Subject: Re: [PATCH 1/2] staging: wilc1000: use 'interrupts' property instead of 'irq-gpio'
Date: Fri, 6 Mar 2020 09:35:13 -0600	[thread overview]
Message-ID: <CAL_JsqKP-9bBMpkvMfphyAfdeVd44=pYDSOY=fSsx5xYkJPCjA@mail.gmail.com> (raw)
In-Reply-To: <20200306144400.4269-2-ajay.kathat@microchip.com>

On Fri, Mar 6, 2020 at 8:44 AM <Ajay.Kathat@microchip.com> wrote:
>
> From: Ajay Singh <ajay.kathat@microchip.com>
>
> 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 <ajay.kathat@microchip.com>
> ---
>  .../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 000000000000..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 <adham.abozaeid@microchip.com>
> +  - Ajay Singh <ajay.kathat@microchip.com>
> +
> +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 = <48000000>;
> +        reg = <0>;
> +        interrupt-parent = <&pioC>;
> +        interrupts = <27 0>;
> +        clocks = <&pck1>;
> +        clock-names = "rtc";
> +      };
> +    };
> +
> +  - |
> +    mmc1: mmc@fc000000 {
> +      #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