Alexis Lothoré <alexis.lothore@xxxxxxxxxxx> writes:
> Hello David,
> just reacting to your answers, but I will take a look at your updated patch
>
> On 1/22/24 21:41, David Mosberger-Tang wrote:
>> Alexis,
>>
>> Thanks for your feedback!
>>
>> On Mon, 2024-01-22 at 15:19 +0100, Alexis Lothoré wrote:
>>> Hello,
>>>
>>> On 1/19/24 22:51, David Mosberger-Tang wrote:
>
> [...]
>
>>>> + if (ret) {
>>>> + ret = -ENODEV;
>>>
>>> I would keep wilc_spi_configure_bus_protocol original error instead of
>>> rewriting/forcing it to -ENODEV here. I mean, if something fails in
>>> wilc_spi_configure_bus_protocol but not right at the first attempt to
>>> communicate with the chip, it does not translate automatically to an absence of
>>> chip, right ?
>>
>> Hmmh, I'm happy to change it, but, as it happens, all failure returns in
>> wilc_spi_configure_bus_protocol() basically mean that the device isn't present
>> or a device is present which the driver doesn't support, so I think -ENODEV is
>> more useful than returning -EINVAL (as would be the case). Let me know if you
>> still think I should change it.
>
> Yeah, but then I would change wilc_spi_configure_bus_protocol() to return
> -ENODEV instead of -EINVAL, if that's really the cause, and just let calling
> functions propagate it. That may just be a personal taste, but I find it pretty
> tedious to debug some error code and eventually realize that some intermediate
> function just rewrote the real error to another one (and sometime, loosing some
> info while doing so).
Yeah, changing error values is very much discouraged.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
