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

From: <Ajay.Kathat@microchip.com>
To: <robh@kernel.org>
Cc: <devel@driverdev.osuosl.org>, <devicetree@vger.kernel.org>,
	<gregkh@linuxfoundation.org>, <linux-wireless@vger.kernel.org>,
	<Adham.Abozaeid@microchip.com>, <johannes@sipsolutions.net>
Subject: Re: [PATCH 1/2] staging: wilc1000: use 'interrupts' property instead of 'irq-gpio'
Date: Sat, 7 Mar 2020 08:50:53 +0000	[thread overview]
Message-ID: <4408ce4a-1914-ae73-e3e0-15cf2d283a6c@microchip.com> (raw)
In-Reply-To: <CAL_JsqKP-9bBMpkvMfphyAfdeVd44=pYDSOY=fSsx5xYkJPCjA@mail.gmail.com>

Hi Rob,

On 06/03/20 9:05 pm, Rob Herring wrote:
> 
> 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.
> 

Ok. I will add them in 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
> 
Ok. I will changes this.

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

Ok.

>> +    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?

The binding file wrongly got added in this patch. It needs to be added
only to '/driver/staging/wilc1000'. I will update this and send v2
series by keeping binding patch separate from code changes.

Regards,
Ajay