Received-SPF: pass (google.com: domain of linux-kernel+bounces-107118-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <c85c3d7c-df50-4328-b260-753a036c92d8@gmail.com>
Date: Tue, 19 Mar 2024 12:12:37 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 1/5] dt-bindings: misc: Add mikrobus-connector
Content-Language: en-US
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
 open list <linux-kernel@vger.kernel.org>
Cc: jkridner@beagleboard.org, robertcnelson@beagleboard.org,
 lorforlinux@beagleboard.org, Rob Herring <robh@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>, Nishanth Menon <nm@ti.com>,
 Vignesh Raghavendra <vigneshr@ti.com>, Tero Kristo <kristo@kernel.org>,
 Derek Kiernan <derek.kiernan@amd.com>, Dragan Cvetic
 <dragan.cvetic@amd.com>, Arnd Bergmann <arnd@arndb.de>,
 Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
 Vaishnav M A <vaishnav.a@ti.com>, Mark Brown <broonie@kernel.org>,
 Johan Hovold <johan@kernel.org>, Alex Elder <elder@kernel.org>,
 "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
 <devicetree@vger.kernel.org>,
 "moderated list:ARM/TEXAS INSTRUMENTS K3 ARCHITECTURE"
 <linux-arm-kernel@lists.infradead.org>,
 "open list:SPI SUBSYSTEM" <linux-spi@vger.kernel.org>,
 "moderated list:GREYBUS SUBSYSTEM" <greybus-dev@lists.linaro.org>,
 Vaishnav M A <vaishnav@beagleboard.org>,
 "Russell King (Oracle)" <linux@armlinux.org.uk>
References: <20240317193714.403132-1-ayushdevel1325@gmail.com>
 <20240317193714.403132-2-ayushdevel1325@gmail.com>
 <1be08004-fe29-4e0b-b0c9-7cc7af15d0b0@linaro.org>
From: Ayush Singh <ayushdevel1325@gmail.com>
In-Reply-To: <1be08004-fe29-4e0b-b0c9-7cc7af15d0b0@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


On 3/19/24 11:33, Krzysztof Kozlowski wrote:
> On 17/03/2024 20:37, Ayush Singh wrote:
>> Add DT bindings for mikroBUS interface. MikroBUS is an open standard
>> developed by MikroElektronika for connecting add-on boards to
>> microcontrollers or microprocessors.
>>
> ...
>
>> +title: mikroBUS add-on board socket
>> +
>> +maintainers:
>> +  - Ayush Singh <ayushdevel1325@gmail.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: mikrobus-connector
>> +
>> +  pinctrl-0: true
>> +  pinctrl-1: true
>> +  pinctrl-2: true
>> +  pinctrl-3: true
>> +  pinctrl-4: true
>> +  pinctrl-5: true
>> +  pinctrl-6: true
>> +  pinctrl-7: true
>> +  pinctrl-8: true
>> +
>> +  pinctrl-names:
>> +    items:
>> +      - const: default
>> +      - const: pwm_default
>> +      - const: pwm_gpio
>> +      - const: uart_default
>> +      - const: uart_gpio
>> +      - const: i2c_default
>> +      - const: i2c_gpio
>> +      - const: spi_default
>> +      - const: spi_gpio
>> +
>> +  mikrobus-gpios:
>> +    minItems: 11
>> +    maxItems: 12
> I don't see any of the issues resolved which I raised at v3. I think
> Russell pointed that you do not have EEPROM and that some pins are
> optional. You do not allow that.

So this patchset does not contain any EEPROM code. The bindings describe 
mikroBUS connector and not mikroBUS addon board. While it is optional 
for the mikroBUS addon board to not use sone pins (aka NC), the pins 
still exist on the connector on the device side. It is not optional to 
have pins in the host device.

> Plus I don't see him being Cced but he had quite detailed look and
> comments at your patchset, so *you are supposed to Cc* him.
>
> I also do not see Rob's comments fully addressed.
>
> Do not send next versions before resolving previous discusssion.

I apologize, I thought he was on the list by get_maintainers.pl, but it 
seems I was mistaken. I will try to remember going forward.

>> +
>> +  i2c-adapter:
>> +    description: i2c adapter attached to the mikrobus socket.
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +
>> +  spi-controller:
>> +    description: spi bus number of the spi-master attached to the mikrobus socket.
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +
>> +  uart:
>> +    description: uart port attached to the mikrobus socket
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +
>> +  pwms:
>> +    description: the pwm-controller corresponding to the mikroBUS PWM pin.
>> +    maxItems: 1
>> +
>> +  spi-cs:
>> +    description: spi chip-select numbers corresponding to the chip-selects on the mikrobus socket.
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    items:
>> +      - description: chip select corresponding to CS pin
>> +      - description: chip select corresponding to RST pin
>> +
>> +required:
>> +  - compatible
>> +  - pinctrl-0
>> +  - pinctrl-1
>> +  - pinctrl-2
>> +  - pinctrl-3
>> +  - pinctrl-4
>> +  - pinctrl-5
>> +  - pinctrl-6
>> +  - pinctrl-7
>> +  - pinctrl-8
>> +  - i2c-adapter
>> +  - spi-controller
>> +  - spi-cs
>> +  - uart
>> +  - pwms
>> +  - mikrobus-gpios
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +
>> +    mikrobus {
>> +      compatible = "mikrobus-connector";
>> +      pinctrl-names = "default", "pwm_default", "pwm_gpio","uart_default", "uart_gpio", "i2c_default",
> Please properly wrap your code according to Linux and DTS coding style
> documents.
>
>
> Best regards,
> Krzysztof
>
Ayush Singh