RE: [PATCH V2 05/13] pm80xx : Support for char device.

Deepak.Ukey
2020-01-22T08:50:22Z

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

On 21/01/2020 05:33, Deepak.Ukey@microchip.com wrote:
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> the content is safe
>
> Martin,
>
>> Thanks for the commit message, looks much better. In the past, people
>> are against IOCTL, suggesting netlink, have you considered that?
>
> Not so keen on adding more ioctls. It's 2020 and all...
>
> Given the nature of the exported information, what's wrong with putting it in sysfs?
> -- We have some upcoming patches which uses this IOCTL interface and that cannot be supported through sysfs.
> Below are the patches in this patchset which requires IOCTL interface.
> 0007-pm80xx-IOCTL-functionality-to-get-phy-status
> 0008-pm80xx-IOCTL-functionality-to-get-phy-error

Please note that there definitely seems to be replication of what sysfs already provides in some of these patches:

- 0007-pm80xx-IOCTL-functionality-to-get-phy-status gets things like Programmed Link Rate, Negotiated Link Rate, PHY Identifier

- 0008-pm80xx-IOCTL-functionality-to-get-phy-error provides other things like Invalid Dword Error Count, Disparity Error Count

See ***:

root@ubuntu:/sys/class/sas_phy/phy-0:0# ls -l total 0
lrwxrwxrwx 1 root root 0 Jan 21 12:05 device -> ../../../phy-0:0
-r--r--r-- 1 root root 4096 Jan 21 12:05 device_type
-rw-r--r-- 1 root root 4096 Jan 21 12:05 enable ***
--w------- 1 root root 4096 Jan 21 12:05 hard_reset
-r--r--r-- 1 root root 4096 Jan 21 12:05 initiator_port_protocols
-r--r--r-- 1 root root 4096 Jan 21 12:05 invalid_dword_count ***
--w------- 1 root root 4096 Jan 21 12:05 link_reset
-r--r--r-- 1 root root 4096 Jan 21 12:05 loss_of_dword_sync_count ***
-rw-r--r-- 1 root root 4096 Jan 21 12:05 maximum_linkrate ***
-r--r--r-- 1 root root 4096 Jan 21 12:05 maximum_linkrate_hw ***
-rw-r--r-- 1 root root 4096 Jan 21 12:05 minimum_linkrate ***
-r--r--r-- 1 root root 4096 Jan 21 12:05 minimum_linkrate_hw ***
-r--r--r-- 1 root root 4096 Jan 21 12:05 negotiated_linkrate ***
-r--r--r-- 1 root root 4096 Jan 21 11:58 phy_identifier ***
-r--r--r-- 1 root root 4096 Jan 21 12:05 phy_reset_problem_count ***
drwxr-xr-x 2 root root 0 Jan 21 12:05 power
-r--r--r-- 1 root root 4096 Jan 21 12:05 running_disparity_error_count ***
-r--r--r-- 1 root root 4096 Jan 21 12:05 sas_address
lrwxrwxrwx 1 root root 0 Jan 21 11:45 subsystem ->
../../../../../../../class/sas_phy
-r--r--r-- 1 root root 4096 Jan 21 12:05 target_port_protocols
-rw-r--r-- 1 root root 4096 Jan 21 11:45 uevent

Maybe the other stuff provided in the patches are useful, I don't know.
But debugfs seems better for that.

- 0006-pm80xx-sysfs-attribute-for-number-of-phys
- 0007-pm80xx-IOCTL-functionality-to-get-phy-status gets things like Programmed Link Rate, Negotiated Link Rate, PHY Identifier
- 0008-pm80xx-IOCTL-functionality-to-get-phy-error provides other things like Invalid Dword Error Count, Disparity Error Count
- Thanks for addressing it. We can get this info from /sys/class/sas_phy and /sys/class/sas_port so we will drop these above mentioned three patches from the next - patch series.

> 0009-pm80xx-IOCTL-functionality-for-GPIO
> 0010-pm80xx-IOCTL-functionality-for-SGPIO

I don't know why an ioctl is required here.

> 0013-pm80xx-IOCTL-functionality-for-TWI-device

- 0009-pm80xx-IOCTL-functionality-for-GPIO
- 0010-pm80xx-IOCTL-functionality-for-SGPIO
- 0013-pm80xx-IOCTL-functionality-for-TWI-device
- For the above patches management utility passes command specific information to driver through IOCTL structure, which used by driver to frame the command and - send to FW. We are using the IOCTL interface for the same. Please let us know your thought.

Regards,
Deepak