Re: [PATCH net-next v3 02/12] net: ethernet: oa_tc6: implement register write operation

[Parthiban.Veerasooran]

Hi Andrew,

On 06/03/24 7:10 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
>> +config OA_TC6
>> +     tristate "OPEN Alliance TC6 10BASE-T1x MAC-PHY support"
>> +     depends on SPI
>> +     select PHYLIB
>> +     help
>> +       This library implements OPEN Alliance TC6 10BASE-T1x MAC-PHY
>> +       Serial Interface protocol for supporting 10BASE-T1x MAC-PHYs.
>> +
>> +       To know the implementation details, refer documentation in
>> +       <file:Documentation/networking/oa-tc6-framework.rst>.
>> +
>> +       This option is provided for the case where no in-kernel-tree modules
>> +       require OA_TC6 functions, but a module built outside the kernel tree
>> +       does. Such modules that use library OA_TC6 functions require M here.
> 
> We generally don't refer to out of tree modules. We know they exist,
> but we don't take any steps to support them, the internal APIs are not
> fixed etc. So i would drop this last paragraph.
Ah ok, sure I will drop this last three lines paragraph in the next version.
> 
>> +static int oa_tc6_check_ctrl_write_reply(struct oa_tc6 *tc6, u8 size)
>> +{
>> +     u8 *tx_buf = tc6->spi_ctrl_tx_buf;
>> +     u8 *rx_buf = tc6->spi_ctrl_rx_buf;
>> +
>> +     rx_buf += OA_TC6_CTRL_IGNORED_SIZE;
>> +
>> +     /* The echoed control write must match with the one that was
>> +      * transmitted.
>> +      */
>> +     if (memcmp(tx_buf, rx_buf, size - OA_TC6_CTRL_IGNORED_SIZE))
>> +             return -ENODEV;
>> +
> 
> I think EPROTO or EIO would be better. The device might have crashed,
> burned and is gone, but isn't a bit flip on the SPI bus more likely?
Yes, it results bit flip in the SPI. So I think it leads to "Protocol 
error". EPROTO would be a better option here. I will correct it in the 
next version.

Best regards,
Parthiban V
> 
>         Andrew
>