Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <28da9e33-57e8-7ac1-7e6c-13c297a945d6@gmail.com>
Date:   Mon, 2 Jan 2023 11:37:40 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH 1/8] spi: dt-bindings: Introduce spi-cs-setup-ns property
Content-Language: en-US
To:     Mark Brown <broonie@kernel.org>, Michael Walle <michael@walle.cc>
Cc:     tudor.ambarus@microchip.com, alexandre.belloni@bootlin.com,
        claudiu.beznea@microchip.com, devicetree@vger.kernel.org,
        krzysztof.kozlowski+dt@linaro.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-mtd@lists.infradead.org, linux-spi@vger.kernel.org,
        nicolas.ferre@microchip.com, robh+dt@kernel.org
References: <20221117105249.115649-2-tudor.ambarus@microchip.com>
 <20221118141458.954646-1-michael@walle.cc> <Y3elIdM3Xz1H4kKk@sirena.org.uk>
From:   Tudor Ambarus <tudor.ambarus@gmail.com>
In-Reply-To: <Y3elIdM3Xz1H4kKk@sirena.org.uk>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

On 18.11.2022 17:30, Mark Brown wrote:
> On Fri, Nov 18, 2022 at 03:14:58PM +0100, Michael Walle wrote:
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
>>> +  spi-cs-setup-ns:
>>> +    description:
>>> +      Delay in nanosecods to be introduced by the controller after CS is
>>> +      asserted.
> 
>> Does this need a type as the spi-cs-setup-ns is apparently just 16bit? At
>> least the driver uses it that way.
> 
>> But IMHO this should just be a normal uint32 value to be consistent with
>> all the other properties. Also the max value with 16bit will be 'just'
>> 65us.
> 
> Making it 32 bit does seem safer.  I've applied the series

Thanks. There are few implications to consider before making this prop a
u32, and I'd like to check them with you.

struct spi_delay will have to be updated to have a u32 value, now it's a
u16. This means that we'll have to update spi_delay_to_ns() to either
return a s64 or to add a u64 *delay parameter to the function so that we
can still handle the conversions from usecs and the error codes in the
SPI_DELAY_UNIT_SCK case. Then all its callers have to be updated to
consider the u64 delay.

I don't know what to say, I'm in between. 65us delays are improbable,
but I'm fine to update this as well. Let me know your preference.

Thanks,
ta