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

From: <Deepak.Ukey@microchip.com>
To: <john.garry@huawei.com>, <martin.petersen@oracle.com>,
	<jinpu.wang@cloud.ionos.com>
Cc: <jejb@linux.ibm.com>, <linux-scsi@vger.kernel.org>,
	<Vasanthalakshmi.Tharmarajan@microchip.com>,
	<Viswas.G@microchip.com>, <jinpu.wang@profitbricks.com>,
	<yuuzheng@google.com>, <auradkar@google.com>,
	<vishakhavc@google.com>, <bjashnani@google.com>,
	<radha@google.com>, <akshatzen@google.com>
Subject: RE: [PATCH V2 05/13] pm80xx : Support for char device.
Date: Tue, 28 Jan 2020 09:43:49 +0000	[thread overview]
Message-ID: <MN2PR11MB35505927EB38FFD749E535CBEF0A0@MN2PR11MB3550.namprd11.prod.outlook.com> (raw)
In-Reply-To: <3e76b6e1-9c3d-2e5c-896e-f1af9a785fe5@huawei.com>


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

On 22/01/2020 08:50, Deepak.Ukey@microchip.com wrote:
> -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.

So I specifically questioned the SGPIO patch and why it would have an IOCTL, as this function is supported in kernel libsas/SAS transport code as an SMP function.
>  Thank you for your suggestions. We will make use of function supported in libsas.

For the GPIO IOCTL, could you use register a gpio driver to provide a gpiolib sysfs?
> We cannot use GPIO driver to provide gpiolib sysfs. There are 24 GPIO signals pin provided by the SPCV controller out of which 16 signals pin are used by customer.  The host application perform different operations on that pins for example pin setup, event setup and read/write GPIO pins.
> For this, applications passes different combination of values to execute the specific operation with help of a payload structure and application passes that structure to driver using IOCTL.
> Driver fetch the information like input enable pin setup, typepart1/typepart2 pin setup, gpio event level setup, gpio rising / falling edge setup, gpio pin mask setup  passed by application and then send with specific command format to execute the gpio operation.

As for TWI, it seems to be for serial EEPROM, so you could ask these experts about how to handle it properly in the kernel for standard sysfs
interfaces:
> Driver supports different TWI devices not limited to the Serial EEPROM. Application passes the address of the attached TWI device to read and write the TWI binary along with the payload structure which contains information like offset, address mode, read/write size.
> Drivers fetch the information passed by application and then send with specific command format to execute operation.

:~/linux$ ./scripts/get_maintainer.pl -f drivers/misc/eeprom/eeprom.c Jean Delvare <jdelvare@suse.com> (maintainer:LEGACY EEPROM DRIVER) Arnd Bergmann <arnd@arndb.de> (supporter:CHAR and MISC DRIVERS) Greg Kroah-Hartman <gregkh@linuxfoundation.org> (supporter:CHAR and MISC
DRIVERS)

Thanks,
Deepak