Re: [PATCH] Add IDT 89HPESx EEPROM/CSR driver
On Sun, Oct 30, 2016 at 09:53:58AM -0400, Greg KH wrote: > On Mon, Oct 03, 2016 at 02:13:45AM +0300, Serge Semin wrote: > > Hello linux folks, > > > > This driver primarily is developed to give an access to EEPROM of IDT > > PCIe-switches. Such switches provide a simple SMBus interface to perform > > IO-operations from/to EEPROM, which is located at private (also called > > master) > > SMBus of the switches. Using that interface this driver creates a simple > > binary sysfs-file in the device directory: > > /sys/bus/i2c/devices/-/eeprom > > In case if read-only flag is specified in the device dts-node, user-space > > applications won't be able to write to the EEPROM sysfs-node. > > > > Additionally IDT 89HPESx SMBus interface provides an ability to > > write/read > > data of device CSRs. This driver exposes corresponding sysfs-file to perform > > simple IO operations using that facility for just basic debug purpose. > > If it's for debugging, please put it in debugfs, not in sysfs. sysfs > files for one-off drivers is usually discouraged, but at the least, you > have to document it in a Documentation/ABI/ file. For debugfs files, we > don't care, you can do whatever you want in them :) > Undrestood. I'll move it to Debugfs. > > Particularly next file is created in the device specific sysfs-directory: > > /sys/bus/i2c/devices/-/csr > > Format of the sysfs-node is: > > $ cat /sys/bus/i2c/devices/-/csr; > > : > > So reading the content of the sysfs-file gives current CSR address and > > it value. If user-space application wishes to change current CSR address, > > it can just write a proper value to the sysfs-file: > > $ echo "" > /sys/bus/i2c/devices/-/csr > > If it wants to change the CSR value as well, the format of the write > > operation is: > > $ echo ":" > \ > > /sys/bus/i2c/devices/-/csr; > > CSR address and value can be any of hexadecimal, decimal or octal format. > > Yeah, that all should go into debugfs. > Ok. > > The driver supports the most of the commonly available SMBus operations: > > SMBus i2c block, SMBus block, smbus word and byte. The code has been tested > > to be built for x32/x64 MIPS architecture and tested on the x32 MIPS > > machine. > > The patch was applied on top of commit > > c6935931c1894ff857616ff8549b61236a19148f > > of master branch of repository > > git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git > > > > > > Thanks, > > > > = > > Serge V. Semin > > Leading Programmer > > Embedded SW development group > > T-platforms > > = > > > > Signed-off-by: Serge Semin > > meta-comment, the "hello", and "thanks" and signature doesn't need to be > in a changelog text, next time you can drop that. See the many kernel > patches on the mailing lists for examples of how to do this. > Understood. > > > > > --- > > .../devicetree/bindings/misc/idt_89hpesx.txt | 41 + > > > Can you split this out into a separate file and be sure to cc: the patch > to the devicetree maintainers? > > > drivers/misc/eeprom/Kconfig| 10 + > > drivers/misc/eeprom/Makefile |1 + > > drivers/misc/eeprom/idt_89hpesx.c | 1483 > > > > 4 files changed, 1535 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/misc/idt_89hpesx.txt > > create mode 100644 drivers/misc/eeprom/idt_89hpesx.c > > > > diff --git a/Documentation/devicetree/bindings/misc/idt_89hpesx.txt > > b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt > > new file mode 100644 > > index 000..469cc93 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt > > @@ -0,0 +1,41 @@ > > +EEPROM / CSR SMBus-slave interface of IDT 89HPESx devices > > + > > +Required properties: > > + - compatible : should be "," > > +Basically there is only one manufacturer: idt, but some > > +compatible devices may be produced in future. Following devices > > +are supported: 89hpes8nt2, 89hpes12nt3, 89hpes24nt6ag2, > > +89hpes32nt8ag2, 89hpes32nt8bg2, 89hpes12nt12g2, 89hpes16nt16g2, > > +89hpes24nt24g2, 89hpes32nt24ag2, 89hpes32nt24bg2; > > +89hpes12n3, 89hpes12n3a, 89hpes24n3, 89hpes24n3a; > > +89hpes32h8, 89hpes32h8g2, 89hpes48h12, 89hpes48h12g2, > > +89hpes48h12ag2, 89hpes16h16, 89hpes22h16, 89hpes22h16g2, > > +89hpes34h16, 89hpes34h16g2, 89hpes64h16, 89hpes64h16g2, > > +89hpes64h16ag2; > > +89hpes12t3g2, 89hpes24t3g2, 89hpes16t4, 89hpes4t4g2, > > +89hpes10t4g2, 89hpes16t4g2, 89hpes16t4ag2, 89hpes5t5, > > +89hpes6t5, 89hpes8t5, 89hpes8t5a, 89hpes24t6, 89hpes6t6g2, > > +89hpes24t6g2, 89hpes16t7, 89hpes32t8, 89hpes32t8g2, > > +89hpes48t12, 89hpes48t12g2. > > +Current implementation of the driver doesn't have any device-
Re: [PATCH] Add IDT 89HPESx EEPROM/CSR driver
On Mon, Oct 03, 2016 at 02:13:45AM +0300, Serge Semin wrote: > Hello linux folks, > > This driver primarily is developed to give an access to EEPROM of IDT > PCIe-switches. Such switches provide a simple SMBus interface to perform > IO-operations from/to EEPROM, which is located at private (also called master) > SMBus of the switches. Using that interface this driver creates a simple > binary sysfs-file in the device directory: > /sys/bus/i2c/devices/-/eeprom > In case if read-only flag is specified in the device dts-node, user-space > applications won't be able to write to the EEPROM sysfs-node. > > Additionally IDT 89HPESx SMBus interface provides an ability to write/read > data of device CSRs. This driver exposes corresponding sysfs-file to perform > simple IO operations using that facility for just basic debug purpose. If it's for debugging, please put it in debugfs, not in sysfs. sysfs files for one-off drivers is usually discouraged, but at the least, you have to document it in a Documentation/ABI/ file. For debugfs files, we don't care, you can do whatever you want in them :) > Particularly next file is created in the device specific sysfs-directory: > /sys/bus/i2c/devices/-/csr > Format of the sysfs-node is: > $ cat /sys/bus/i2c/devices/-/csr; > : > So reading the content of the sysfs-file gives current CSR address and > it value. If user-space application wishes to change current CSR address, > it can just write a proper value to the sysfs-file: > $ echo "" > /sys/bus/i2c/devices/-/csr > If it wants to change the CSR value as well, the format of the write > operation is: > $ echo ":" > \ > /sys/bus/i2c/devices/-/csr; > CSR address and value can be any of hexadecimal, decimal or octal format. Yeah, that all should go into debugfs. > The driver supports the most of the commonly available SMBus operations: > SMBus i2c block, SMBus block, smbus word and byte. The code has been tested > to be built for x32/x64 MIPS architecture and tested on the x32 MIPS machine. > The patch was applied on top of commit > c6935931c1894ff857616ff8549b61236a19148f > of master branch of repository > git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git > > > Thanks, > > = > Serge V. Semin > Leading Programmer > Embedded SW development group > T-platforms > = > > Signed-off-by: Serge Semin meta-comment, the "hello", and "thanks" and signature doesn't need to be in a changelog text, next time you can drop that. See the many kernel patches on the mailing lists for examples of how to do this. > > --- > .../devicetree/bindings/misc/idt_89hpesx.txt | 41 + Can you split this out into a separate file and be sure to cc: the patch to the devicetree maintainers? > drivers/misc/eeprom/Kconfig| 10 + > drivers/misc/eeprom/Makefile |1 + > drivers/misc/eeprom/idt_89hpesx.c | 1483 > > 4 files changed, 1535 insertions(+) > create mode 100644 Documentation/devicetree/bindings/misc/idt_89hpesx.txt > create mode 100644 drivers/misc/eeprom/idt_89hpesx.c > > diff --git a/Documentation/devicetree/bindings/misc/idt_89hpesx.txt > b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt > new file mode 100644 > index 000..469cc93 > --- /dev/null > +++ b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt > @@ -0,0 +1,41 @@ > +EEPROM / CSR SMBus-slave interface of IDT 89HPESx devices > + > +Required properties: > + - compatible : should be "," > + Basically there is only one manufacturer: idt, but some > + compatible devices may be produced in future. Following devices > + are supported: 89hpes8nt2, 89hpes12nt3, 89hpes24nt6ag2, > + 89hpes32nt8ag2, 89hpes32nt8bg2, 89hpes12nt12g2, 89hpes16nt16g2, > + 89hpes24nt24g2, 89hpes32nt24ag2, 89hpes32nt24bg2; > + 89hpes12n3, 89hpes12n3a, 89hpes24n3, 89hpes24n3a; > + 89hpes32h8, 89hpes32h8g2, 89hpes48h12, 89hpes48h12g2, > + 89hpes48h12ag2, 89hpes16h16, 89hpes22h16, 89hpes22h16g2, > + 89hpes34h16, 89hpes34h16g2, 89hpes64h16, 89hpes64h16g2, > + 89hpes64h16ag2; > + 89hpes12t3g2, 89hpes24t3g2, 89hpes16t4, 89hpes4t4g2, > + 89hpes10t4g2, 89hpes16t4g2, 89hpes16t4ag2, 89hpes5t5, > + 89hpes6t5, 89hpes8t5, 89hpes8t5a, 89hpes24t6, 89hpes6t6g2, > + 89hpes24t6g2, 89hpes16t7, 89hpes32t8, 89hpes32t8g2, > + 89hpes48t12, 89hpes48t12g2. > + Current implementation of the driver doesn't have any device- > + specific functionalities. But since each of them differs > + by registers mapping, CSRs read/write restrictions can be > + added in future. > + - reg : I2C address of the IDT 89HPES device. > + > +Optional properties: > + - read-only : Parameterless property
[PATCH] Add IDT 89HPESx EEPROM/CSR driver
Hello linux folks, This driver primarily is developed to give an access to EEPROM of IDT PCIe-switches. Such switches provide a simple SMBus interface to perform IO-operations from/to EEPROM, which is located at private (also called master) SMBus of the switches. Using that interface this driver creates a simple binary sysfs-file in the device directory: /sys/bus/i2c/devices/-/eeprom In case if read-only flag is specified in the device dts-node, user-space applications won't be able to write to the EEPROM sysfs-node. Additionally IDT 89HPESx SMBus interface provides an ability to write/read data of device CSRs. This driver exposes corresponding sysfs-file to perform simple IO operations using that facility for just basic debug purpose. Particularly next file is created in the device specific sysfs-directory: /sys/bus/i2c/devices/-/csr Format of the sysfs-node is: $ cat /sys/bus/i2c/devices/-/csr; : So reading the content of the sysfs-file gives current CSR address and it value. If user-space application wishes to change current CSR address, it can just write a proper value to the sysfs-file: $ echo "" > /sys/bus/i2c/devices/-/csr If it wants to change the CSR value as well, the format of the write operation is: $ echo ":" > \ /sys/bus/i2c/devices/-/csr; CSR address and value can be any of hexadecimal, decimal or octal format. The driver supports the most of the commonly available SMBus operations: SMBus i2c block, SMBus block, smbus word and byte. The code has been tested to be built for x32/x64 MIPS architecture and tested on the x32 MIPS machine. The patch was applied on top of commit c6935931c1894ff857616ff8549b61236a19148f of master branch of repository git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git Thanks, = Serge V. Semin Leading Programmer Embedded SW development group T-platforms = Signed-off-by: Serge Semin --- .../devicetree/bindings/misc/idt_89hpesx.txt | 41 + drivers/misc/eeprom/Kconfig| 10 + drivers/misc/eeprom/Makefile |1 + drivers/misc/eeprom/idt_89hpesx.c | 1483 4 files changed, 1535 insertions(+) create mode 100644 Documentation/devicetree/bindings/misc/idt_89hpesx.txt create mode 100644 drivers/misc/eeprom/idt_89hpesx.c diff --git a/Documentation/devicetree/bindings/misc/idt_89hpesx.txt b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt new file mode 100644 index 000..469cc93 --- /dev/null +++ b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt @@ -0,0 +1,41 @@ +EEPROM / CSR SMBus-slave interface of IDT 89HPESx devices + +Required properties: + - compatible : should be "," +Basically there is only one manufacturer: idt, but some +compatible devices may be produced in future. Following devices +are supported: 89hpes8nt2, 89hpes12nt3, 89hpes24nt6ag2, +89hpes32nt8ag2, 89hpes32nt8bg2, 89hpes12nt12g2, 89hpes16nt16g2, +89hpes24nt24g2, 89hpes32nt24ag2, 89hpes32nt24bg2; +89hpes12n3, 89hpes12n3a, 89hpes24n3, 89hpes24n3a; +89hpes32h8, 89hpes32h8g2, 89hpes48h12, 89hpes48h12g2, +89hpes48h12ag2, 89hpes16h16, 89hpes22h16, 89hpes22h16g2, +89hpes34h16, 89hpes34h16g2, 89hpes64h16, 89hpes64h16g2, +89hpes64h16ag2; +89hpes12t3g2, 89hpes24t3g2, 89hpes16t4, 89hpes4t4g2, +89hpes10t4g2, 89hpes16t4g2, 89hpes16t4ag2, 89hpes5t5, +89hpes6t5, 89hpes8t5, 89hpes8t5a, 89hpes24t6, 89hpes6t6g2, +89hpes24t6g2, 89hpes16t7, 89hpes32t8, 89hpes32t8g2, +89hpes48t12, 89hpes48t12g2. +Current implementation of the driver doesn't have any device- +specific functionalities. But since each of them differs +by registers mapping, CSRs read/write restrictions can be +added in future. + - reg : I2C address of the IDT 89HPES device. + +Optional properties: + - read-only : Parameterless property disables writes to the EEPROM + - idt,eesize : Size of EEPROM device connected to IDT 89HPES i2c-master bus +(default value is 4096 bytes if option isn't specified) + - idt,eeaddr : Custom address of EEPROM device +(If not specified IDT 89HPESx device will try to communicate + with EEPROM sited by default address - 0x50) + +Example: + idt_pcie_sw@60 { + compatible = "idt,89hpes12nt3"; + reg = <0x60>; + read-only; + idt,eesize = <65536>; + idt,eeaddr = <0x50>; + }; diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig index c4e41c2..de58762 100644 --- a/drivers/misc/eeprom/Kconfig +++ b/drivers/misc/eeprom/Kconfig @@ -100,4 +100,14 @@ config EEPROM_DIGSY_MTC_CFG