AW: [PATCH v2 2/3] phy: Realtek Otto SerDes driver

[Krzysztof Kozlowski]

On 11/10/2024 18:10, markus.stockhausen at gmx.de wrote:
>>> +
>>> +	ret = kstrtou32_from_user(userbuf, count, 10, &reset);
>>> +	if (ret || reset != 1)
>>> +		return ret;
>>> +
>>> +	rtsds_phy_reset_int(ctrl, sid);
>>> +
>>> +	return count;
>>> +}
>>> +DEFINE_SHOW_STORE_ATTRIBUTE(rtsds_dbg_reset);
>>
>> That's not a debugfs interface. Drop reset.
>>
> 
> Hi Krzysztof,
> 
> thanks for all your feedback and kickstarting me into this all. I went back to 
> the drawing board. A overhauled version will be available over the weekend.
> Regarding the debug interface I have a question left:
> 
> The current state of exploration and understanding of the four different SoCs
> boils down that 3 debug writing functions will improve further testing and
> development very much.
> 
> - reset SerDes -> used to compensate for link hangs (because of wrong firmware)
> - change mode -> I will change it so it will only accept supported modes
> - modify registers -> I can go without that.
> 

You can always have it as out of tree.

> One may ask why? There are a lot of different devices out there and I have only
> one of each series. So if the driver provides this debugging flexibility users
> (especially of OpenWrt) can assist in analyzing/testing without complete rebuilds.

I do not oppose it strongly, but I think it's risky even if you allow it
only to the root user. How does the driver handle resets happening in
random moments, e.g. shortly after probe? It feels like a way to brick
the device or make it inoperable, if root is malicious.

For some reason `git grep SHOW_STORE | grep reset` gives exactly 0
results... :)

> 
> What will be the way forward here without dropping the code?

You can still send v3 with the code with argumentation why debug
interface should be able to perform reset of serdes phy.

> 
> Last but not least: I expect the v3 version to have ~1/3 of the code changed.
> Shall I still give this a v3 tag?

Yes

Best regards,
Krzysztof