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

Parthiban.Veerasooran
2024-03-07T06:46:56Z

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
>