Date:   Thu, 24 Nov 2016 21:42:02 +0300
From:   Serge Semin <fancer.lancer@...il.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     arnd@...db.de, wsa@...-dreams.de, jdelvare@...e.com,
        linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org,
        Sergey.Semin@...latforms.ru
Subject: Re: [PATCH] Add IDT 89HPESx EEPROM/CSR driver

On Sun, Oct 30, 2016 at 09:53:58AM -0400, Greg KH <gregkh@...uxfoundation.org> 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/<bus>-<devaddr>/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/<bus>-<devaddr>/csr
> > Format of the sysfs-node is:
> > $ cat /sys/bus/i2c/devices/<bus>-<devaddr>/csr;
> > <CSR address>:<CSR value>
> > 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 "<CSR address>" > /sys/bus/i2c/devices/<bus>-<devaddr>/csr
> > If it wants to change the CSR value as well, the format of the write
> > operation is:
> > $ echo "<CSR address>:<CSR value>" > \
> >         /sys/bus/i2c/devices/<bus>-<devaddr>/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 <fancer.lancer@...il.com>
> 
> 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 0000000..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 "<manufacturer>,<type>"
> > +		 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
> >  
> >  	  If unsure, say N.
> >  
> > +config EEPROM_IDT_89HPESX
> > +	tristate "IDT 89HPESx PCIe-swtiches EEPROM / CSR support"
> > +	depends on I2C && SYSFS
> > +	help
> > +	  Enable this driver to get read/write access to EEPROM / CSRs
> > +	  over IDT PCIe-swtich i2c-slave interface.
> > +
> > +	  This driver can also be built as a module. If so, the module
> > +	  will be called idt_89hpesx.
> > +
> >  endmenu
> > diff --git a/drivers/misc/eeprom/Makefile b/drivers/misc/eeprom/Makefile
> > index fc1e81d..90a5262 100644
> > --- a/drivers/misc/eeprom/Makefile
> > +++ b/drivers/misc/eeprom/Makefile
> > @@ -5,3 +5,4 @@ obj-$(CONFIG_EEPROM_MAX6875)	+= max6875.o
> >  obj-$(CONFIG_EEPROM_93CX6)	+= eeprom_93cx6.o
> >  obj-$(CONFIG_EEPROM_93XX46)	+= eeprom_93xx46.o
> >  obj-$(CONFIG_EEPROM_DIGSY_MTC_CFG) += digsy_mtc_eeprom.o
> > +obj-$(CONFIG_EEPROM_IDT_89HPESX) += idt_89hpesx.o
> > diff --git a/drivers/misc/eeprom/idt_89hpesx.c b/drivers/misc/eeprom/idt_89hpesx.c
> > new file mode 100644
> > index 0000000..f95f4f0
> > --- /dev/null
> > +++ b/drivers/misc/eeprom/idt_89hpesx.c
> > @@ -0,0 +1,1483 @@
> > +/*
> > + *   This file is provided under a GPLv2 license.  When using or
> > + *   redistributing this file, you may do so under that license.
> > + *
> > + *   GPL LICENSE SUMMARY
> > + *
> > + *   Copyright (C) 2016 T-Platforms All Rights Reserved.
> > + *
> > + *   This program is free software; you can redistribute it and/or modify it
> > + *   under the terms and conditions of the GNU General Public License,
> > + *   version 2, as published by the Free Software Foundation.
> > + *
> > + *   This program is distributed in the hope that it will be useful, but WITHOUT
> > + *   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + *   FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> > + *   more details.
> > + *
> > + *   You should have received a copy of the GNU General Public License along
> > + *   with this program; if not, it can be found <http://www.gnu.org/licenses/>.
> > + *
> > + *   The full GNU General Public License is included in this distribution in
> > + *   the file called "COPYING".
> > + *
> > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> > + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> > + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> > + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > + *
> > + * IDT PCIe-switch NTB Linux driver
> > + *
> > + * Contact Information:
> > + * Serge Semin <fancer.lancer@...il.com>, <Sergey.Semin@...latforms.ru>
> > + */
> > +/*
> > + *           NOTE of the IDT 89HPESx SMBus-slave interface driver
> > + *    This driver primarily is developed to have an access to EEPROM device of
> > + * IDT PCIe-switches. IDT provides a simple SMBus interface to perform IO-
> > + * operations from/to EEPROM, which is located at private (so called Master)
> > + * SMBus of switches. Using that interface this the driver creates a simple
> > + * binary sysfs-file in the device directory:
> > + * /sys/bus/i2c/devices/<bus>-<devaddr>/eeprom
> > + * In case if read-only flag is specified in the dts-node of device desription,
> > + * User-space applications won't be able to write to the EEPROM sysfs-node.
> > + *    Additionally IDT 89HPESx SMBus interface has an ability to write/read
> > + * data of device CSRs. This driver exposes another sysfs-file to perform
> > + * simple IO operations using that ability for just basic debug purpose.
> > + * Particularly next file is created in the device specific sysfs-directory:
> > + * /sys/bus/i2c/devices/<bus>-<devaddr>/csr
> > + * Format of the sysfs-node is:
> > + * $ cat /sys/bus/i2c/devices/<bus>-<devaddr>/csr;
> > + * <CSR address>:<CSR value>
> > + * 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 "<CSR address>" > /sys/bus/i2c/devices/<bus>-<devaddr>/csr
> > + * If it wants to change the CSR value as well, the format of the write
> > + * operation is:
> > + * $ echo "<CSR address>:<CSR value>" > \
> > + *        /sys/bus/i2c/devices/<bus>-<devaddr>/csr;
> > + * CSR address and value can be any of hexadecimal, decimal or octal format.
> > + */
> > +
> > +/*
> > + * Note: You can load this module with either option 'dyndbg=+p' or define the
> > + * next preprocessor constant
> 
> What does this option do?  I don't understand it.
> 
> > + */
> > +/* #define DEBUG */
> 
> Ah, just drop all of this, dynamic debugging is known how to be used
> through the whole kernel, no need to document it again :)
> 

Ok. I'll drop all of this down.

> > +
> > +#include <linux/kernel.h>
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/types.h>
> > +#include <linux/sizes.h>
> > +#include <linux/slab.h>
> > +#include <linux/mutex.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/of.h>
> > +#include <linux/i2c.h>
> > +#include <linux/pci_ids.h>
> 
> why pci_ids.h?
> 

I need to have PCI ids to probe the IDT device. As I already said IDT PCIe
switch has a separate i2c slave interface with an access to all device CSRs,
which are basically PCI configuration spaces of all switch ports. So when
device is probed, the driver reads VID and DID to make sure, that it is
really connected to IDT switch.

> > +
> > +#define IDT_NAME		"89hpesx"
> > +#define IDT_89HPESX_DESC	"IDT 89HPESx SMBus-slave interface driver"
> > +#define IDT_89HPESX_VER		"1.0"
> > +
> > +MODULE_DESCRIPTION(IDT_89HPESX_DESC);
> > +MODULE_VERSION(IDT_89HPESX_VER);
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_AUTHOR("T-platforms");
> > +
> > +/*
> > + * Some common constant used in the driver for better readability:
> > + * @SUCCESS:	Success of a function execution
> > + */
> > +#define SUCCESS (0)
> 
> No need to define this, we all know that 0 is "success", this is the
> kernel.
> 

Ok, I'll delete it from the code.

> > +
> > +/*
> > + * struct idt_89hpesx_dev - IDT 89HPESx device data structure
> > + * @eesize:	Size of EEPROM in bytes (calculated from "idt,eecompatible")
> > + * @eero:	EEPROM Read-only flag
> > + * @eeaddr:	EEPROM custom address
> > + *
> > + * @inieecmd:	Initial cmd value for EEPROM read/write operations
> > + * @inicsrcmd:	Initial cmd value for CSR read/write operations
> > + * @iniccode:	Initialial command code value for IO-operations
> > + *
> > + * @csr:	CSR address to perform read operation
> > + *
> > + * @smb_write:	SMBus write method
> > + * @smb_read:	SMBus read method
> > + * @smb_mtx:	SMBus mutex
> > + *
> > + * @client:	i2c client used to perform IO operations
> > + *
> > + * @eenode:	EEPROM sysfs-node to read/write data to/from EEPROM
> > + * @regnode:	Register sysfs-node to read/write CSRs
> > + */
> > +struct idt_smb_seq;
> > +struct idt_89hpesx_dev {
> > +	u32 eesize;
> > +	bool eero;
> > +	u8 eeaddr;
> > +
> > +	u8 inieecmd;
> > +	u8 inicsrcmd;
> > +	u8 iniccode;
> > +
> > +	atomic_t csr;
> > +
> > +	int (*smb_write)(struct idt_89hpesx_dev *, const struct idt_smb_seq *);
> > +	int (*smb_read)(struct idt_89hpesx_dev *, struct idt_smb_seq *);
> > +	struct mutex smb_mtx;
> > +
> > +	struct i2c_client *client;
> > +
> > +	struct bin_attribute eenode;
> > +	struct bin_attribute regnode;
> > +};
> > +#define to_pdev_kobj(__kobj) \
> > +	dev_get_drvdata(container_of(__kobj, struct device, kobj))
> > +
> > +/*
> > + * struct idt_smb_seq - sequence of data to be read/written from/to IDT 89HPESx
> > + * @ccode:	SMBus command code
> > + * @bytecnt:	Byte count of operation
> > + * @data:	Data to by written
> > + */
> > +struct idt_smb_seq {
> > +	u8 ccode;
> > +	u8 bytecnt;
> > +	u8 *data;
> > +};
> > +
> > +/*
> > + * struct idt_eeprom_seq - sequence of data to be read/written from/to EEPROM
> > + * @cmd:	Transaction CMD
> > + * @eeaddr:	EEPROM custom address
> > + * @memaddr:	Internal memory address of EEPROM
> > + * @data:	Data to be written at the memory address
> > + */
> > +struct idt_eeprom_seq {
> > +	u8 cmd;
> > +	u8 eeaddr;
> > +	u16 memaddr;
> 
> endian?  Is this big or little?
> 

Since the IDT-switch is PCIe device, it's little endian. Of course, the code
does the proper conversions. You can see it in the functions:
idt_eeprom_write()/idt_eeprom_read().

> > +	u8 data;
> > +} __packed;
> > +
> > +/*
> > + * struct idt_csr_seq - sequence of data to be read/written from/to CSR
> > + * @cmd:	Transaction CMD
> > + * @csraddr:	Internal IDT device CSR address
> > + * @data:	Data to be read/written from/to the CSR address
> > + */
> > +struct idt_csr_seq {
> > +	u8 cmd;
> > +	u16 csraddr;
> > +	u32 data;
> 
> Same for these, what endian are they?
> 
> > +} __packed;
> > +
> > +/*
> > + * SMBus command code macros
> > + * @CCODE_END:		Indicates the end of transaction
> > + * @CCODE_START:	Indicates the start of transaction
> > + * @CCODE_CSR:		CSR read/write transaction
> > + * @CCODE_EEPROM:	EEPROM read/write transaction
> > + * @CCODE_BYTE:		Supplied data has BYTE length
> > + * @CCODE_WORD:		Supplied data has WORD length
> > + * @CCODE_BLOCK:	Supplied data has variable length passed in bytecnt
> > + *			byte right following CCODE byte
> > + */
> > +#define CCODE_END	((u8)0x01)
> > +#define CCODE_START	((u8)0x02)
> > +#define CCODE_CSR	((u8)0x00)
> > +#define CCODE_EEPROM	((u8)0x04)
> > +#define CCODE_BYTE	((u8)0x00)
> > +#define CCODE_WORD	((u8)0x20)
> > +#define CCODE_BLOCK	((u8)0x40)
> > +#define CCODE_PEC	((u8)0x80)
> > +
> > +/*
> > + * EEPROM command macros
> > + * @EEPROM_OP_WRITE:	EEPROM write operation
> > + * @EEPROM_OP_READ:	EEPROM read operation
> > + * @EEPROM_USA:		Use specified address of EEPROM
> > + * @EEPROM_NAERR:	EEPROM device is not ready to respond
> > + * @EEPROM_LAERR:	EEPROM arbitration loss error
> > + * @EEPROM_MSS:		EEPROM misplace start & stop bits error
> > + * @EEPROM_WR_CNT:	Bytes count to perform write operation
> > + * @EEPROM_WRRD_CNT:	Bytes count to write before reading
> > + * @EEPROM_RD_CNT:	Bytes count to perform read operation
> > + * @EEPROM_DEF_SIZE:	Fall back size of EEPROM
> > + * @EEPROM_DEF_ADDR:	Defatul EEPROM address
> > + */
> > +#define EEPROM_OP_WRITE	((u8)0x00)
> > +#define EEPROM_OP_READ	((u8)0x01)
> > +#define EEPROM_USA	((u8)0x02)
> > +#define EEPROM_NAERR	((u8)0x08)
> > +#define EEPROM_LAERR    ((u8)0x10)
> > +#define EEPROM_MSS	((u8)0x20)
> > +#define EEPROM_WR_CNT	((u8)5)
> > +#define EEPROM_WRRD_CNT	((u8)4)
> > +#define EEPROM_RD_CNT	((u8)5)
> > +#define EEPROM_DEF_SIZE	((u16)4096)
> > +#define EEPROM_DEF_ADDR	((u8)0x50)
> > +
> > +/*
> > + * CSR command macros
> > + * @CSR_DWE:		Enable all four bytes of the operation
> > + * @CSR_OP_WRITE:	CSR write operation
> > + * @CSR_OP_READ:	CSR read operation
> > + * @CSR_RERR:		Read operation error
> > + * @CSR_WERR:		Write operation error
> > + * @CSR_WR_CNT:		Bytes count to perform write operation
> > + * @CSR_WRRD_CNT:	Bytes count to write before reading
> > + * @CSR_RD_CNT:		Bytes count to perform read operation
> > + * @CSR_MAX:		Maximum CSR address
> > + * @CSR_DEF:		Default CSR address
> > + * @CSR_REAL_ADDR:	CSR real unshifted address
> > + */
> > +#define CSR_DWE			((u8)0x0F)
> > +#define CSR_OP_WRITE		((u8)0x00)
> > +#define CSR_OP_READ		((u8)0x10)
> > +#define CSR_RERR		((u8)0x40)
> > +#define CSR_WERR		((u8)0x80)
> > +#define CSR_WR_CNT		((u8)7)
> > +#define CSR_WRRD_CNT		((u8)3)
> > +#define CSR_RD_CNT		((u8)7)
> > +#define CSR_MAX			((u32)0x3FFFF)
> > +#define CSR_DEF			((u16)0x0000)
> > +#define CSR_REAL_ADDR(val)	((unsigned int)val << 2)
> > +
> > +/*
> > + * IDT 89HPESx basic register
> > + * @IDT_VIDDID_CSR:	PCIe VID and DID of IDT 89HPESx
> > + * @IDT_VID_MASK:	Mask of VID
> > + */
> > +#define IDT_VIDDID_CSR	((u32)0x0000)
> > +#define IDT_VID_MASK	((u32)0xFFFF)
> > +
> > +/*
> > + * IDT 89HPESx can send NACK when new command is sent before previous one
> > + * fininshed execution. In this case driver retries operation
> > + * certain times.
> > + * @RETRY_CNT:		Number of retries before giving up and fail
> > + * @idt_smb_safe:	Generate a retry loop on corresponding SMBus method
> > + */
> > +#define RETRY_CNT (128)
> > +#define idt_smb_safe(ops, args...) ({ \
> > +	int __retry = RETRY_CNT; \
> > +	s32 __sts; \
> > +	do { \
> > +		__sts = i2c_smbus_ ## ops ## _data(args); \
> > +	} while (__retry-- && __sts < SUCCESS); \
> > +	__sts; \
> > +})
> > +
> > +/*
> > + * Wrapper dev_err/dev_warn/dev_info/dev_dbg macros
> > + */
> > +#define dev_err_idt(pdev, args...) \
> > +	dev_err(&pdev->client->dev, ## args)
> > +#define dev_warn_idt(pdev, args...) \
> > +	dev_warn(&pdev->client->dev, ## args)
> > +#define dev_info_idt(pdev, args...) \
> > +	dev_info(&pdev->client->dev, ## args)
> > +#define dev_dbg_idt(pdev, args...) \
> > +	dev_dbg(&pdev->client->dev, ## args)
> 
> ick, don't wrap them, just use them as-is, it's not that hard to write
> them out please.
> 

Ok.

> > +
> > +/*===========================================================================
> > + *                         i2c bus level IO-operations
> > + *===========================================================================
> > + */
> > +
> > +/*
> > + * idt_smb_write_byte() - SMBus write method when I2C_SMBUS_BYTE_DATA operation
> > + *                        is only available
> > + * @pdev:	Pointer to the driver data
> > + * @seq:	Sequence of data to be written
> > + */
> > +static int idt_smb_write_byte(struct idt_89hpesx_dev *pdev,
> > +			      const struct idt_smb_seq *seq)
> > +{
> > +	s32 sts;
> > +	u8 ccode;
> > +	int idx;
> > +
> > +	/* Loop over the supplied data sending byte one-by-one */
> > +	for (idx = 0; idx < seq->bytecnt; idx++) {
> > +		/* Collect the command code byte */
> > +		ccode = seq->ccode | CCODE_BYTE;
> > +		if (idx == 0)
> > +			ccode |= CCODE_START;
> > +		if (idx == seq->bytecnt - 1)
> > +			ccode |= CCODE_END;
> > +
> > +		/* Send data to the device */
> > +		sts = idt_smb_safe(write_byte, pdev->client, ccode,
> > +			seq->data[idx]);
> > +		if (sts != SUCCESS)
> > +			return (int)sts;
> > +	}
> > +
> > +	return SUCCESS;
> > +}
> > +
> > +/*
> > + * idt_smb_read_byte() - SMBus read method when I2C_SMBUS_BYTE_DATA operation
> > + *                        is only available
> > + * @pdev:	Pointer to the driver data
> > + * @seq:	Buffer to read data to
> > + */
> > +static int idt_smb_read_byte(struct idt_89hpesx_dev *pdev,
> > +			     struct idt_smb_seq *seq)
> > +{
> > +	s32 sts;
> > +	u8 ccode;
> > +	int idx;
> > +
> > +	/* Loop over the supplied buffer receiving byte one-by-one */
> > +	for (idx = 0; idx < seq->bytecnt; idx++) {
> > +		/* Collect the command code byte */
> > +		ccode = seq->ccode | CCODE_BYTE;
> > +		if (idx == 0)
> > +			ccode |= CCODE_START;
> > +		if (idx == seq->bytecnt - 1)
> > +			ccode |= CCODE_END;
> > +
> > +		/* Read data from the device */
> > +		sts = idt_smb_safe(read_byte, pdev->client, ccode);
> > +		if (sts < SUCCESS)
> > +			return (int)sts;
> > +
> > +		seq->data[idx] = (u8)sts;
> > +	}
> > +
> > +	return SUCCESS;
> > +}
> > +
> > +/*
> > + * idt_smb_write_word() - SMBus write method when I2C_SMBUS_BYTE_DATA and
> > + *                        I2C_FUNC_SMBUS_WORD_DATA operations are available
> > + * @pdev:	Pointer to the driver data
> > + * @seq:	Sequence of data to be written
> > + */
> > +static int idt_smb_write_word(struct idt_89hpesx_dev *pdev,
> > +			      const struct idt_smb_seq *seq)
> > +{
> > +	s32 sts;
> > +	u8 ccode;
> > +	int idx, evencnt;
> > +
> > +	/* Calculate the even count of data to send */
> > +	evencnt = seq->bytecnt - (seq->bytecnt % 2);
> > +
> > +	/* Loop over the supplied data sending two bytes at a time */
> > +	for (idx = 0; idx < evencnt; idx += 2) {
> > +		/* Collect the command code byte */
> > +		ccode = seq->ccode | CCODE_WORD;
> > +		if (idx == 0)
> > +			ccode |= CCODE_START;
> > +		if (idx == evencnt - 2)
> > +			ccode |= CCODE_END;
> > +
> > +		/* Send word data to the device */
> > +		sts = idt_smb_safe(write_word, pdev->client, ccode,
> > +			*(u16 *)&seq->data[idx]);
> > +		if (sts != SUCCESS)
> > +			return (int)sts;
> > +	}
> > +
> > +	/* If there is odd number of bytes then send just one last byte */
> > +	if (seq->bytecnt != evencnt) {
> > +		/* Collect the command code byte */
> > +		ccode = seq->ccode | CCODE_BYTE | CCODE_END;
> > +		if (idx == 0)
> > +			ccode |= CCODE_START;
> > +
> > +		/* Send byte data to the device */
> > +		sts = idt_smb_safe(write_byte, pdev->client, ccode,
> > +			seq->data[idx]);
> > +		if (sts != SUCCESS)
> > +			return (int)sts;
> > +	}
> > +
> > +	return SUCCESS;
> > +}
> > +
> > +/*
> > + * idt_smb_read_word() - SMBus read method when I2C_SMBUS_BYTE_DATA and
> > + *                       I2C_FUNC_SMBUS_WORD_DATA operations are available
> > + * @pdev:	Pointer to the driver data
> > + * @seq:	Buffer to read data to
> > + */
> > +static int idt_smb_read_word(struct idt_89hpesx_dev *pdev,
> > +			     struct idt_smb_seq *seq)
> > +{
> > +	s32 sts;
> > +	u8 ccode;
> > +	int idx, evencnt;
> > +
> > +	/* Calculate the even count of data to send */
> > +	evencnt = seq->bytecnt - (seq->bytecnt % 2);
> > +
> > +	/* Loop over the supplied data reading two bytes at a time */
> > +	for (idx = 0; idx < evencnt; idx += 2) {
> > +		/* Collect the command code byte */
> > +		ccode = seq->ccode | CCODE_WORD;
> > +		if (idx == 0)
> > +			ccode |= CCODE_START;
> > +		if (idx == evencnt - 2)
> > +			ccode |= CCODE_END;
> > +
> > +		/* Read word data from the device */
> > +		sts = idt_smb_safe(read_word, pdev->client, ccode);
> > +		if (sts < SUCCESS)
> > +			return (int)sts;
> > +
> > +		*(u16 *)&seq->data[idx] = (u16)sts;
> > +	}
> > +
> > +	/* If there is odd number of bytes then receive just one last byte */
> > +	if (seq->bytecnt != evencnt) {
> > +		/* Collect the command code byte */
> > +		ccode = seq->ccode | CCODE_BYTE | CCODE_END;
> > +		if (idx == 0)
> > +			ccode |= CCODE_START;
> > +
> > +		/* Read last data byte from the device */
> > +		sts = idt_smb_safe(read_byte, pdev->client, ccode);
> > +		if (sts < SUCCESS)
> > +			return (int)sts;
> > +
> > +		seq->data[idx] = (u8)sts;
> > +	}
> > +
> > +	return SUCCESS;
> > +}
> > +
> > +/*
> > + * idt_smb_write_block() - SMBus write method when I2C_SMBUS_BLOCK_DATA
> > + *                         operation is available
> > + * @pdev:	Pointer to the driver data
> > + * @seq:	Sequence of data to be written
> > + */
> > +static int idt_smb_write_block(struct idt_89hpesx_dev *pdev,
> > +			       const struct idt_smb_seq *seq)
> > +{
> > +	u8 ccode;
> > +
> > +	/* Return error if too much data passed to send */
> > +	if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
> > +		return -EINVAL;
> > +
> > +	/* Collect the command code byte */
> > +	ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
> > +
> > +	/* Send block of data to the device */
> > +	return idt_smb_safe(write_block, pdev->client, ccode, seq->bytecnt,
> > +		seq->data);
> > +}
> > +
> > +/*
> > + * idt_smb_read_block() - SMBus read method when I2C_SMBUS_BLOCK_DATA
> > + *                        operation is available
> > + * @pdev:	Pointer to the driver data
> > + * @seq:	Buffer to read data to
> > + */
> > +static int idt_smb_read_block(struct idt_89hpesx_dev *pdev,
> > +			      struct idt_smb_seq *seq)
> > +{
> > +	s32 sts;
> > +	u8 ccode;
> > +
> > +	/* Return error if too much data passed to send */
> > +	if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
> > +		return -EINVAL;
> > +
> > +	/* Collect the command code byte */
> > +	ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
> > +
> > +	/* Read block of data from the device */
> > +	sts = idt_smb_safe(read_block, pdev->client, ccode, seq->data);
> > +	if (sts != seq->bytecnt)
> > +		return (sts < SUCCESS ? sts : -ENODATA);
> > +
> > +	return SUCCESS;
> > +}
> > +
> > +/*
> > + * idt_smb_write_i2c_block() - SMBus write method when I2C_SMBUS_I2C_BLOCK_DATA
> > + *                             operation is available
> > + * @pdev:	Pointer to the driver data
> > + * @seq:	Sequence of data to be written
> > + *
> > + * NOTE It's usual SMBus write block operation, except the actual data length is
> > + * sent as first byte of data
> > + */
> > +static int idt_smb_write_i2c_block(struct idt_89hpesx_dev *pdev,
> > +				   const struct idt_smb_seq *seq)
> > +{
> > +	u8 ccode, buf[I2C_SMBUS_BLOCK_MAX + 1];
> > +
> > +	/* Return error if too much data passed to send */
> > +	if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
> > +		return -EINVAL;
> > +
> > +	/* Collect the data to send. Length byte must be added prior the data */
> > +	buf[0] = seq->bytecnt;
> > +	memcpy(&buf[1], seq->data, seq->bytecnt);
> > +
> > +	/* Collect the command code byte */
> > +	ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
> > +
> > +	/* Send length and block of data to the device */
> > +	return idt_smb_safe(write_i2c_block, pdev->client, ccode,
> > +		seq->bytecnt + 1, buf);
> > +}
> > +
> > +/*
> > + * idt_smb_read_i2c_block() - SMBus read method when I2C_SMBUS_I2C_BLOCK_DATA
> > + *                            operation is available
> > + * @pdev:	Pointer to the driver data
> > + * @seq:	Buffer to read data to
> > + *
> > + * NOTE It's usual SMBus read block operation, except the actual data length is
> > + * retrieved as first byte of data
> > + */
> > +static int idt_smb_read_i2c_block(struct idt_89hpesx_dev *pdev,
> > +				  struct idt_smb_seq *seq)
> > +{
> > +	u8 ccode, buf[I2C_SMBUS_BLOCK_MAX + 1];
> > +	s32 sts;
> > +
> > +	/* Return error if too much data passed to send */
> > +	if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
> > +		return -EINVAL;
> > +
> > +	/* Collect the command code byte */
> > +	ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
> > +
> > +	/* Read length and block of data from the device */
> > +	sts = idt_smb_safe(read_i2c_block, pdev->client, ccode,
> > +		seq->bytecnt + 1, buf);
> > +	if (sts != seq->bytecnt + 1)
> > +		return (sts < SUCCESS ? sts : -ENODATA);
> > +	if (buf[0] != seq->bytecnt)
> > +		return -ENODATA;
> > +
> > +	/* Copy retrieved data to the output data buffer */
> > +	memcpy(seq->data, &buf[1], seq->bytecnt);
> > +
> > +	return SUCCESS;
> > +}
> > +
> > +/*===========================================================================
> > + *                          EEPROM IO-operations
> > + *===========================================================================
> > + */
> > +
> > +/*
> > + * idt_eeprom_write() - EEPROM write operation
> > + * @pdev:	Pointer to the driver data
> > + * @memaddr:	Start EEPROM memory address
> > + * @len:	Length of data to be written
> > + * @data:	Data to be written to EEPROM
> > + */
> > +static int idt_eeprom_write(struct idt_89hpesx_dev *pdev, u16 memaddr, u16 len,
> > +			    const u8 *data)
> > +{
> > +	struct idt_eeprom_seq eeseq;
> > +	struct idt_smb_seq smbseq;
> > +	int ret, retry;
> > +	u16 idx;
> > +
> > +	/* Initialize SMBus sequence fields */
> > +	smbseq.ccode = pdev->iniccode | CCODE_EEPROM;
> > +	smbseq.data = (u8 *)&eeseq;
> > +
> > +	/* Send data byte-by-byte, checking if it is successfully written */
> > +	for (idx = 0; idx < len; idx++, memaddr++) {
> > +		/* Lock IDT SMBus device */
> > +		mutex_lock(&pdev->smb_mtx);
> > +
> > +		/* Perform write operation */
> > +		smbseq.bytecnt = EEPROM_WR_CNT;
> > +		eeseq.cmd = pdev->inieecmd | EEPROM_OP_WRITE;
> > +		eeseq.eeaddr = pdev->eeaddr;
> > +		eeseq.memaddr = cpu_to_le16(memaddr);
> > +		eeseq.data = data[idx];
> > +		ret = pdev->smb_write(pdev, &smbseq);
> > +		if (ret != SUCCESS) {
> > +			dev_err_idt(pdev,
> > +				"Failed to write 0x%04hx:0x%02hhx to eeprom",
> > +				memaddr, data[idx]);
> > +			goto err_mutex_unlock;
> > +		}
> > +
> > +		/*
> > +		 * Check whether the data is successfully written by reading
> > +		 * from the same EEPROM memory address. Sometimes EEPROM may
> > +		 * respond with NACK if it's busy with writing previous data,
> > +		 * so we need to perform a few attempts of read cycle
> > +		 */
> > +		retry = RETRY_CNT;
> > +		do {
> > +			/* Send EEPROM memory address to read data from */
> > +			smbseq.bytecnt = EEPROM_WRRD_CNT;
> > +			eeseq.cmd = pdev->inieecmd | EEPROM_OP_READ;
> > +			ret = pdev->smb_write(pdev, &smbseq);
> > +			if (ret != SUCCESS) {
> > +				dev_err_idt(pdev,
> > +					"Failed to init mem address 0x%02hhx",
> > +					memaddr);
> > +				goto err_mutex_unlock;
> > +			}
> > +
> > +			/* Perform read operation */
> > +			smbseq.bytecnt = EEPROM_RD_CNT;
> > +			eeseq.data = ~data[idx];
> > +			ret = pdev->smb_read(pdev, &smbseq);
> > +			if (ret != SUCCESS) {
> > +				dev_err_idt(pdev,
> > +					"Failed to read mem address 0x%02hhx",
> > +					memaddr);
> > +				goto err_mutex_unlock;
> > +			}
> > +		} while (retry-- && (eeseq.cmd & EEPROM_NAERR));
> > +
> > +		/* Check whether IDT successfully sent data to EEPROM */
> > +		if (eeseq.cmd & (EEPROM_NAERR | EEPROM_LAERR | EEPROM_MSS)) {
> > +			dev_err_idt(pdev, "Communication with EEPROM failed");
> > +			ret = -EREMOTEIO;
> > +			goto err_mutex_unlock;
> > +		}
> > +		if (eeseq.data != data[idx]) {
> > +			dev_err_idt(pdev,
> > +				"Values don't match 0x%02hhx != 0x%02hhx",
> > +				eeseq.data, data[idx]);
> > +			ret = -EREMOTEIO;
> > +			goto err_mutex_unlock;
> > +		}
> > +
> > +		/* Unlock IDT SMBus device */
> > +err_mutex_unlock:
> > +		mutex_unlock(&pdev->smb_mtx);
> > +		if (ret != SUCCESS)
> > +			return ret;
> > +	}
> > +
> > +	return SUCCESS;
> > +}
> > +
> > +/*
> > + * idt_eeprom_read() - EEPROM read operation
> > + * @pdev:	Pointer to the driver data
> > + * @memaddr:	Start EEPROM memory address
> > + * @len:	Length of data to read
> > + * @buf:	Buffer to read data to
> > + */
> > +static int idt_eeprom_read(struct idt_89hpesx_dev *pdev, u16 memaddr, u16 len,
> > +			   u8 *buf)
> > +{
> > +	struct idt_eeprom_seq eeseq;
> > +	struct idt_smb_seq smbseq;
> > +	u16 idx;
> > +	int ret;
> > +
> > +	/* Initialize SMBus sequence fields */
> > +	smbseq.ccode = pdev->iniccode | CCODE_EEPROM;
> > +	smbseq.data = (u8 *)&eeseq;
> > +
> > +	/* Send data one-by-one, checking if it is successfully written */
> > +	for (idx = 0; idx < len; idx++, memaddr++) {
> > +		/* Lock IDT SMBus device */
> > +		mutex_lock(&pdev->smb_mtx);
> > +
> > +		/* Send EEPROM memory address to read data from */
> > +		smbseq.bytecnt = EEPROM_WRRD_CNT;
> > +		eeseq.cmd = pdev->inieecmd | EEPROM_OP_READ;
> > +		eeseq.eeaddr = pdev->eeaddr;
> > +		eeseq.memaddr = cpu_to_le16(memaddr);
> > +		ret = pdev->smb_write(pdev, &smbseq);
> > +		if (ret != SUCCESS) {
> > +			dev_err_idt(pdev, "Failed to init mem address 0x%02hhx",
> > +				memaddr);
> > +			goto err_mutex_unlock;
> > +		}
> > +
> > +		/* Perform read operation (rest of fields stay the same) */
> > +		smbseq.bytecnt = EEPROM_RD_CNT;
> > +		ret = pdev->smb_read(pdev, &smbseq);
> > +		if (ret != SUCCESS) {
> > +			dev_err_idt(pdev,
> > +				"Failed to read eeprom address 0x%02hhx",
> > +				memaddr);
> > +			ret = -EREMOTEIO;
> > +			goto err_mutex_unlock;
> > +		}
> > +
> > +		/* Check whether IDT successfully read data from EEPROM */
> > +		if (eeseq.cmd & (EEPROM_NAERR | EEPROM_LAERR | EEPROM_MSS)) {
> > +			dev_err_idt(pdev, "Communication with eeprom failed");
> > +			ret = -EREMOTEIO;
> > +			goto err_mutex_unlock;
> > +		}
> > +
> > +		/* Save retrieved data */
> > +		buf[idx] = eeseq.data;
> > +
> > +		/* Unlock IDT SMBus device */
> > +err_mutex_unlock:
> > +		mutex_unlock(&pdev->smb_mtx);
> > +		if (ret != SUCCESS)
> > +			return ret;
> > +	}
> > +
> > +	return SUCCESS;
> > +}
> > +
> > +/*===========================================================================
> > + *                          CSR IO-operations
> > + *===========================================================================
> > + */
> > +
> > +/*
> > + * idt_csr_write() - CSR write operation
> > + * @pdev:	Pointer to the driver data
> > + * @csraddr:	CSR address (with no two LS bits)
> > + * @data:	Data to be written to CSR
> > + */
> > +static int idt_csr_write(struct idt_89hpesx_dev *pdev, u16 csraddr,
> > +			 const u32 data)
> > +{
> > +	struct idt_csr_seq csrseq;
> > +	struct idt_smb_seq smbseq;
> > +	int ret;
> > +
> > +	/* Initialize SMBus sequence fields */
> > +	smbseq.ccode = pdev->iniccode | CCODE_CSR;
> > +	smbseq.data = (u8 *)&csrseq;
> > +
> > +	/* Lock IDT SMBus device */
> > +	mutex_lock(&pdev->smb_mtx);
> > +
> > +	/* Perform write operation */
> > +	smbseq.bytecnt = CSR_WR_CNT;
> > +	csrseq.cmd = pdev->inicsrcmd | CSR_OP_WRITE;
> > +	csrseq.csraddr = cpu_to_le16(csraddr);
> > +	csrseq.data = cpu_to_le32(data);
> > +	ret = pdev->smb_write(pdev, &smbseq);
> > +	if (ret != SUCCESS) {
> > +		dev_err_idt(pdev, "Failed to write 0x%04x: 0x%04x to csr",
> > +			CSR_REAL_ADDR(csraddr), data);
> > +		goto err_mutex_unlock;
> > +	}
> > +
> > +	/* Send CSR address to read data from */
> > +	smbseq.bytecnt = CSR_WRRD_CNT;
> > +	csrseq.cmd = pdev->inicsrcmd | CSR_OP_READ;
> > +	ret = pdev->smb_write(pdev, &smbseq);
> > +	if (ret != SUCCESS) {
> > +		dev_err_idt(pdev, "Failed to init csr address 0x%04x",
> > +			CSR_REAL_ADDR(csraddr));
> > +		goto err_mutex_unlock;
> > +	}
> > +
> > +	/* Perform read operation */
> > +	smbseq.bytecnt = CSR_RD_CNT;
> > +	ret = pdev->smb_read(pdev, &smbseq);
> > +	if (ret != SUCCESS) {
> > +		dev_err_idt(pdev, "Failed to read csr 0x%04x",
> > +			CSR_REAL_ADDR(csraddr));
> > +		goto err_mutex_unlock;
> > +	}
> > +
> > +	/* Check whether IDT successfully retrieved CSR data */
> > +	if (csrseq.cmd & (CSR_RERR | CSR_WERR)) {
> > +		dev_err_idt(pdev, "IDT failed to perform CSR r/w");
> > +		ret = -EREMOTEIO;
> > +		goto err_mutex_unlock;
> > +	}
> > +
> > +	/* Unlock IDT SMBus device */
> > +err_mutex_unlock:
> > +	mutex_unlock(&pdev->smb_mtx);
> > +
> > +	return ret;
> > +}
> > +
> > +/*
> > + * idt_csr_read() - CSR read operation
> > + * @pdev:	Pointer to the driver data
> > + * @csraddr:	CSR address (with no two LS bits)
> > + * @data:	Data to be written to CSR
> > + */
> > +static int idt_csr_read(struct idt_89hpesx_dev *pdev, u16 csraddr, u32 *data)
> > +{
> > +	struct idt_csr_seq csrseq;
> > +	struct idt_smb_seq smbseq;
> > +	int ret;
> > +
> > +	/* Initialize SMBus sequence fields */
> > +	smbseq.ccode = pdev->iniccode | CCODE_CSR;
> > +	smbseq.data = (u8 *)&csrseq;
> > +
> > +	/* Lock IDT SMBus device */
> > +	mutex_lock(&pdev->smb_mtx);
> > +
> > +	/* Send CSR register address before reading it */
> > +	smbseq.bytecnt = CSR_WRRD_CNT;
> > +	csrseq.cmd = pdev->inicsrcmd | CSR_OP_READ;
> > +	csrseq.csraddr = cpu_to_le16(csraddr);
> > +	ret = pdev->smb_write(pdev, &smbseq);
> > +	if (ret != SUCCESS) {
> > +		dev_err_idt(pdev, "Failed to init csr address 0x%04x",
> > +			CSR_REAL_ADDR(csraddr));
> > +		goto err_mutex_unlock;
> > +	}
> > +
> > +	/* Perform read operation */
> > +	smbseq.bytecnt = CSR_RD_CNT;
> > +	ret = pdev->smb_read(pdev, &smbseq);
> > +	if (ret != SUCCESS) {
> > +		dev_err_idt(pdev, "Failed to read csr 0x%04hx",
> > +			CSR_REAL_ADDR(csraddr));
> > +		goto err_mutex_unlock;
> > +	}
> > +
> > +	/* Check whether IDT successfully retrieved CSR data */
> > +	if (csrseq.cmd & (CSR_RERR | CSR_WERR)) {
> > +		dev_err_idt(pdev, "IDT failed to perform CSR r/w");
> > +		ret = -EREMOTEIO;
> > +		goto err_mutex_unlock;
> > +	}
> > +
> > +	/* Save data retrieved from IDT */
> > +	*data = le32_to_cpu(csrseq.data);
> > +
> > +	/* Unlock IDT SMBus device */
> > +err_mutex_unlock:
> > +	mutex_unlock(&pdev->smb_mtx);
> > +
> > +	return ret;
> > +}
> > +
> > +/*===========================================================================
> > + *                          Sysfs-nodes IO-operations
> > + *===========================================================================
> > + */
> > +
> > +/*
> > + * idt_sysfs_eeprom_write() - EEPROM sysfs-node write callback
> > + * @filep:	Pointer to the file system node
> > + * @kobj:	Pointer to the kernel object related to the sysfs-node
> > + * @attr:	Attributes of the file
> > + * @buf:	Buffer to write data to
> > + * @off:	Offset at which data should be written to
> > + * @count:	Number of bytes to write
> > + */
> > +static ssize_t idt_sysfs_eeprom_write(struct file *filp, struct kobject *kobj,
> > +				      struct bin_attribute *attr,
> > +				      char *buf, loff_t off, size_t count)
> > +{
> > +	struct idt_89hpesx_dev *pdev;
> > +	int ret;
> > +
> > +	/* Retrieve driver data */
> > +	pdev = to_pdev_kobj(kobj);
> > +
> > +	/* Perform EEPROM write operation */
> > +	ret = idt_eeprom_write(pdev, (u16)off, (u16)count, (u8 *)buf);
> > +	return (ret != SUCCESS ? ret : count);
> > +}
> > +
> > +/*
> > + * idt_sysfs_eeprom_read() - EEPROM sysfs-node read callback
> > + * @filep:	Pointer to the file system node
> > + * @kobj:	Pointer to the kernel object related to the sysfs-node
> > + * @attr:	Attributes of the file
> > + * @buf:	Buffer to write data to
> > + * @off:	Offset at which data should be written to
> > + * @count:	Number of bytes to write
> > + */
> > +static ssize_t idt_sysfs_eeprom_read(struct file *filp, struct kobject *kobj,
> > +				     struct bin_attribute *attr,
> > +				     char *buf, loff_t off, size_t count)
> > +{
> > +	struct idt_89hpesx_dev *pdev;
> > +	int ret;
> > +
> > +	/* Retrieve driver data */
> > +	pdev = to_pdev_kobj(kobj);
> > +
> > +	/* Perform EEPROM read operation */
> > +	ret = idt_eeprom_read(pdev, (u16)off, (u16)count, (u8 *)buf);
> > +	return (ret != SUCCESS ? ret : count);
> > +}
> > +
> > +/*
> > + * idt_sysfs_csr_store() - CSR sysfs-node write callback
> > + * @kobj:	Pointer to the kernel object related to the sysfs-node
> > + * @attr:	Attributes of the file
> > + * @buf:	Buffer to write data to
> > + * @count:	Size of the buffer
> > + *
> > + * It accepts either "0x<reg addr>:0x<value>" for saving register address
> > + * and writing value to specified DWORD register or "0x<reg addr>" for
> > + * just saving register address in order to perform next read operation.
> > + *
> > + * WARNING No spaces are allowed. Incoming string must be strictly formated as:
> > + * "<reg addr>:<value>". Register address must be aligned within 4 bytes
> > + * (one DWORD).
> > + */
> > +static ssize_t idt_sysfs_csr_store(struct device *dev,
> > +				   struct device_attribute *attr,
> > +				   const char *buf, size_t count)
> > +{
> > +	struct idt_89hpesx_dev *pdev;
> > +	char *colon_ch, *csraddr_str, *csrval_str;
> > +	int ret, csraddr_len, csrval_len;
> > +	u32 csraddr, csrval;
> > +
> > +	/* Retrieve driver data */
> > +	pdev = dev_get_drvdata(dev);
> > +
> > +	/* Find position of colon in the buffer */
> > +	colon_ch = strnchr(buf, count, ':');
> > +
> > +	/*
> > +	 * If there is colon passed then new CSR value should be parsed as
> > +	 * well, so allocate buffer for CSR address substring.
> > +	 * If no colon is found, then string must have just one number with
> > +	 * no new CSR value
> > +	 */
> > +	if (colon_ch != NULL) {
> > +		csraddr_len = colon_ch - buf;
> > +		csraddr_str =
> > +			kmalloc(sizeof(char)*(csraddr_len + 1), GFP_KERNEL);
> > +		if (csraddr_str == NULL)
> > +			return -ENOMEM;
> > +		/* Copy the register address to the substring buffer */
> > +		strncpy(csraddr_str, buf, csraddr_len);
> > +		csraddr_str[csraddr_len] = '\0';
> > +		/* Register value must follow the colon */
> > +		csrval_str = colon_ch + 1;
> > +		csrval_len = strnlen(csrval_str, count - csraddr_len);
> > +	} else /* if (str_colon == NULL) */ {
> > +		csraddr_str = (char *)buf; /* Just to shut warning up */
> > +		csraddr_len = strnlen(csraddr_str, count);
> > +		csrval_str = NULL;
> > +		csrval_len = 0;
> > +	}
> > +
> > +	/* Convert CSR address to u32 value */
> > +	ret = kstrtou32(csraddr_str, 0, &csraddr);
> > +	if (ret != SUCCESS)
> > +		goto free_csraddr_str;
> > +
> > +	/* Check whether passed register address is valid */
> > +	if (csraddr > CSR_MAX || !IS_ALIGNED(csraddr, SZ_4)) {
> > +		ret = -EINVAL;
> > +		goto free_csraddr_str;
> > +	}
> > +
> > +	/* Shift register address to the right so to have u16 address */
> > +	csraddr >>= 2;
> > +
> > +	/* Parse new CSR value and send it to IDT, if colon has been found */
> > +	if (colon_ch != NULL) {
> > +		ret = kstrtou32(csrval_str, 0, &csrval);
> > +		if (ret != SUCCESS)
> > +			goto free_csraddr_str;
> > +
> > +		ret = idt_csr_write(pdev, (u16)csraddr, csrval);
> > +		if (ret != SUCCESS)
> > +			goto free_csraddr_str;
> > +	}
> > +
> > +	/* Save CSR address in the data structure for future read operations */
> > +	atomic_set(&pdev->csr, (int)csraddr);
> > +
> > +	/* Free memory only if colon has been found */
> > +free_csraddr_str:
> > +	if (colon_ch != NULL)
> > +		kfree(csraddr_str);
> > +
> > +	return (ret != SUCCESS ? ret : count);
> > +}
> > +
> > +/*
> > + * idt_sysfs_csr_show() - CSR sysfs-node read callback
> > + * @kobj:	Pointer to the kernel object related to the sysfs-node
> > + * @attr:	Attributes of the file
> > + * @buf:	Buffer to write data to
> > + *
> > + * It just prints the pair "0x<reg addr>:0x<value>" to passed buffer.
> > + */
> > +static ssize_t idt_sysfs_csr_show(struct device *dev,
> > +				  struct device_attribute *attr, char *buf)
> > +{
> > +	struct idt_89hpesx_dev *pdev;
> > +	u32 csraddr, csrval;
> > +	int ret;
> > +
> > +	/* Retrieve driver data */
> > +	pdev = dev_get_drvdata(dev);
> > +
> > +	/* Read current CSR address */
> > +	csraddr = atomic_read(&pdev->csr);
> > +
> > +	/* Perform CSR read operation */
> > +	ret = idt_csr_read(pdev, (u16)csraddr, &csrval);
> > +	if (ret != SUCCESS)
> > +		return ret;
> > +
> > +	/* Shift register address to the left so to have real address */
> > +	csraddr <<= 2;
> > +
> > +	/* Print the "0x<reg addr>:0x<value>" to buffer */
> > +	return snprintf(buf, PAGE_SIZE, "0x%05x:0x%08x\n",
> > +		(unsigned int)csraddr, (unsigned int)csrval);
> > +}
> > +
> > +/*
> > + * eeprom_attribute - EEPROM sysfs-node attributes
> > + *
> > + * NOTE Size will be changed in compliance with OF node. EEPROM attribute will
> > + * be read-only as well if the corresponding flag is specified in OF node.
> > + */
> > +static struct bin_attribute eeprom_attribute = {
> > +	.attr = {
> > +		.name = "eeprom",
> > +		.mode = S_IRUGO | S_IWUSR
> > +	},
> > +	.size = EEPROM_DEF_SIZE,
> > +	.write = idt_sysfs_eeprom_write,
> > +	.read = idt_sysfs_eeprom_read
> > +};
> > +
> > +/*
> > + * csr_attribute - CSR sysfs-node attributes
> > + */
> > +static struct device_attribute csr_attribute = {
> > +	.attr = {
> > +		.name = "csr",
> > +		.mode = S_IRUGO | S_IWUSR
> > +	},
> > +	.store = idt_sysfs_csr_store,
> > +	.show = idt_sysfs_csr_show,
> > +};
> 
> Note, for the future, just use DEVICE_ATTR_RW() for stuff like this.
> 
> But convert it to debugfs please.
> 
> thanks,
> 
> greg k-h

Ok. I'll fix all the issues as soon as possible.

Powered by blists - more mailing lists