From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751343AbdEBGYf (ORCPT ); Tue, 2 May 2017 02:24:35 -0400 Received: from mail-yw0-f195.google.com ([209.85.161.195]:34891 "EHLO mail-yw0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751008AbdEBGYd (ORCPT ); Tue, 2 May 2017 02:24:33 -0400 MIME-Version: 1.0 In-Reply-To: <20170410194706.64280-8-cbostic@linux.vnet.ibm.com> References: <20170410194706.64280-1-cbostic@linux.vnet.ibm.com> <20170410194706.64280-8-cbostic@linux.vnet.ibm.com> From: Joel Stanley Date: Tue, 2 May 2017 15:54:11 +0930 X-Google-Sender-Auth: hMRHV-BPmFXsTDU8OtSD2da0WDo Message-ID: Subject: Re: [PATCH v6 07/23] drivers/fsi: Implement slave initialisation To: Christopher Bostic Cc: Rob Herring , Mark Rutland , Russell King , rostedt@goodmis.org, mingo@redhat.com, Greg KH , devicetree , linux-arm-kernel@lists.infradead.org, Jeremy Kerr , Linux Kernel Mailing List , Andrew Jeffery , Alistair Popple , Benjamin Herrenschmidt Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 11, 2017 at 5:16 AM, Christopher Bostic wrote: > From: Jeremy Kerr > > Implement fsi_slave_init: if we can read a chip ID, create fsi_slave > devices and register with the driver core. > > Includes changes from Chris Bostic . > > Signed-off-by: Jeremy Kerr > Signed-off-by: Chris Bostic > Signed-off-by: Joel Stanley > --- > drivers/fsi/fsi-core.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 64 insertions(+), 2 deletions(-) > > diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c > index 6e1cfdf..c705ca2 100644 > --- a/drivers/fsi/fsi-core.c > +++ b/drivers/fsi/fsi-core.c > @@ -17,9 +17,12 @@ > #include > #include > #include > +#include > > #include "fsi-master.h" > > +#define FSI_SLAVE_SIZE_23b 0x800000 > + > static DEFINE_IDA(master_ida); > > struct fsi_slave { > @@ -114,11 +117,70 @@ static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr, > addr, val, size); > } > > +static void fsi_slave_release(struct device *dev) > +{ > + struct fsi_slave *slave = to_fsi_slave(dev); > + > + kfree(slave); > +} > + > static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) > { > - /* todo: initialise slave device, perform engine scan */ > + struct fsi_slave *slave; > + uint32_t chip_id; > + uint8_t crc; > + int rc; > + > + /* Currently, we only support single slaves on a link, and use the > + * full 23-bit address range > + */ > + if (id != 0) > + return -EINVAL; > + > + rc = fsi_master_read(master, link, id, 0, &chip_id, sizeof(chip_id)); > + if (rc) { > + dev_warn(&master->dev, "can't read slave %02x:%02x %d\n", > + link, id, rc); When I boot a system with this driver loaded, I get his warning: [ 9.740000] usbhid: USB HID core driver [ 9.840000] fsi0: can't read slave 00:00 -5 [ 9.840000] NET: Registered protocol family 10 Two things: There's a space in front of "fsi0". This warning isn't useful at that point. The slave is not readable as the FSI master is not present (the P9 hasn't been turned on). Can we avoid printing the warning at boot? Cheers, Joel > + return -ENODEV; > + } > + chip_id = be32_to_cpu(chip_id); > + > + crc = fsi_crc4(0, chip_id, 32); > + if (crc) { > + dev_warn(&master->dev, "slave %02x:%02x invalid chip id CRC!\n", > + link, id); > + return -EIO; > + } > + > + dev_info(&master->dev, "fsi: found chip %08x at %02x:%02x:%02x\n", > + chip_id, master->idx, link, id); > + > + /* We can communicate with a slave; create the slave device and > + * register. > + */ > + slave = kzalloc(sizeof(*slave), GFP_KERNEL); > + if (!slave) > + return -ENOMEM; > + > + slave->master = master; > + slave->dev.parent = &master->dev; > + slave->dev.release = fsi_slave_release; > + slave->link = link; > + slave->id = id; > + slave->size = FSI_SLAVE_SIZE_23b; > + > + dev_set_name(&slave->dev, "slave@%02x:%02x", link, id); > + rc = device_register(&slave->dev); > + if (rc < 0) { > + dev_warn(&master->dev, "failed to create slave device: %d\n", > + rc); > + put_device(&slave->dev); > + return rc; > + } > + > + /* todo: perform engine scan */ > > - return -ENODEV; > + return rc; > } > > /* FSI master support */ > -- > 1.8.2.2 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joel Stanley Subject: Re: [PATCH v6 07/23] drivers/fsi: Implement slave initialisation Date: Tue, 2 May 2017 15:54:11 +0930 Message-ID: References: <20170410194706.64280-1-cbostic@linux.vnet.ibm.com> <20170410194706.64280-8-cbostic@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: <20170410194706.64280-8-cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Christopher Bostic Cc: Rob Herring , Mark Rutland , Russell King , rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org, mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, Greg KH , devicetree , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Jeremy Kerr , Linux Kernel Mailing List , Andrew Jeffery , Alistair Popple , Benjamin Herrenschmidt List-Id: devicetree@vger.kernel.org On Tue, Apr 11, 2017 at 5:16 AM, Christopher Bostic wrote: > From: Jeremy Kerr > > Implement fsi_slave_init: if we can read a chip ID, create fsi_slave > devices and register with the driver core. > > Includes changes from Chris Bostic . > > Signed-off-by: Jeremy Kerr > Signed-off-by: Chris Bostic > Signed-off-by: Joel Stanley > --- > drivers/fsi/fsi-core.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 64 insertions(+), 2 deletions(-) > > diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c > index 6e1cfdf..c705ca2 100644 > --- a/drivers/fsi/fsi-core.c > +++ b/drivers/fsi/fsi-core.c > @@ -17,9 +17,12 @@ > #include > #include > #include > +#include > > #include "fsi-master.h" > > +#define FSI_SLAVE_SIZE_23b 0x800000 > + > static DEFINE_IDA(master_ida); > > struct fsi_slave { > @@ -114,11 +117,70 @@ static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr, > addr, val, size); > } > > +static void fsi_slave_release(struct device *dev) > +{ > + struct fsi_slave *slave = to_fsi_slave(dev); > + > + kfree(slave); > +} > + > static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) > { > - /* todo: initialise slave device, perform engine scan */ > + struct fsi_slave *slave; > + uint32_t chip_id; > + uint8_t crc; > + int rc; > + > + /* Currently, we only support single slaves on a link, and use the > + * full 23-bit address range > + */ > + if (id != 0) > + return -EINVAL; > + > + rc = fsi_master_read(master, link, id, 0, &chip_id, sizeof(chip_id)); > + if (rc) { > + dev_warn(&master->dev, "can't read slave %02x:%02x %d\n", > + link, id, rc); When I boot a system with this driver loaded, I get his warning: [ 9.740000] usbhid: USB HID core driver [ 9.840000] fsi0: can't read slave 00:00 -5 [ 9.840000] NET: Registered protocol family 10 Two things: There's a space in front of "fsi0". This warning isn't useful at that point. The slave is not readable as the FSI master is not present (the P9 hasn't been turned on). Can we avoid printing the warning at boot? Cheers, Joel > + return -ENODEV; > + } > + chip_id = be32_to_cpu(chip_id); > + > + crc = fsi_crc4(0, chip_id, 32); > + if (crc) { > + dev_warn(&master->dev, "slave %02x:%02x invalid chip id CRC!\n", > + link, id); > + return -EIO; > + } > + > + dev_info(&master->dev, "fsi: found chip %08x at %02x:%02x:%02x\n", > + chip_id, master->idx, link, id); > + > + /* We can communicate with a slave; create the slave device and > + * register. > + */ > + slave = kzalloc(sizeof(*slave), GFP_KERNEL); > + if (!slave) > + return -ENOMEM; > + > + slave->master = master; > + slave->dev.parent = &master->dev; > + slave->dev.release = fsi_slave_release; > + slave->link = link; > + slave->id = id; > + slave->size = FSI_SLAVE_SIZE_23b; > + > + dev_set_name(&slave->dev, "slave@%02x:%02x", link, id); > + rc = device_register(&slave->dev); > + if (rc < 0) { > + dev_warn(&master->dev, "failed to create slave device: %d\n", > + rc); > + put_device(&slave->dev); > + return rc; > + } > + > + /* todo: perform engine scan */ > > - return -ENODEV; > + return rc; > } > > /* FSI master support */ > -- > 1.8.2.2 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: joel@jms.id.au (Joel Stanley) Date: Tue, 2 May 2017 15:54:11 +0930 Subject: [PATCH v6 07/23] drivers/fsi: Implement slave initialisation In-Reply-To: <20170410194706.64280-8-cbostic@linux.vnet.ibm.com> References: <20170410194706.64280-1-cbostic@linux.vnet.ibm.com> <20170410194706.64280-8-cbostic@linux.vnet.ibm.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, Apr 11, 2017 at 5:16 AM, Christopher Bostic wrote: > From: Jeremy Kerr > > Implement fsi_slave_init: if we can read a chip ID, create fsi_slave > devices and register with the driver core. > > Includes changes from Chris Bostic . > > Signed-off-by: Jeremy Kerr > Signed-off-by: Chris Bostic > Signed-off-by: Joel Stanley > --- > drivers/fsi/fsi-core.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 64 insertions(+), 2 deletions(-) > > diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c > index 6e1cfdf..c705ca2 100644 > --- a/drivers/fsi/fsi-core.c > +++ b/drivers/fsi/fsi-core.c > @@ -17,9 +17,12 @@ > #include > #include > #include > +#include > > #include "fsi-master.h" > > +#define FSI_SLAVE_SIZE_23b 0x800000 > + > static DEFINE_IDA(master_ida); > > struct fsi_slave { > @@ -114,11 +117,70 @@ static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr, > addr, val, size); > } > > +static void fsi_slave_release(struct device *dev) > +{ > + struct fsi_slave *slave = to_fsi_slave(dev); > + > + kfree(slave); > +} > + > static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) > { > - /* todo: initialise slave device, perform engine scan */ > + struct fsi_slave *slave; > + uint32_t chip_id; > + uint8_t crc; > + int rc; > + > + /* Currently, we only support single slaves on a link, and use the > + * full 23-bit address range > + */ > + if (id != 0) > + return -EINVAL; > + > + rc = fsi_master_read(master, link, id, 0, &chip_id, sizeof(chip_id)); > + if (rc) { > + dev_warn(&master->dev, "can't read slave %02x:%02x %d\n", > + link, id, rc); When I boot a system with this driver loaded, I get his warning: [ 9.740000] usbhid: USB HID core driver [ 9.840000] fsi0: can't read slave 00:00 -5 [ 9.840000] NET: Registered protocol family 10 Two things: There's a space in front of "fsi0". This warning isn't useful at that point. The slave is not readable as the FSI master is not present (the P9 hasn't been turned on). Can we avoid printing the warning at boot? Cheers, Joel > + return -ENODEV; > + } > + chip_id = be32_to_cpu(chip_id); > + > + crc = fsi_crc4(0, chip_id, 32); > + if (crc) { > + dev_warn(&master->dev, "slave %02x:%02x invalid chip id CRC!\n", > + link, id); > + return -EIO; > + } > + > + dev_info(&master->dev, "fsi: found chip %08x at %02x:%02x:%02x\n", > + chip_id, master->idx, link, id); > + > + /* We can communicate with a slave; create the slave device and > + * register. > + */ > + slave = kzalloc(sizeof(*slave), GFP_KERNEL); > + if (!slave) > + return -ENOMEM; > + > + slave->master = master; > + slave->dev.parent = &master->dev; > + slave->dev.release = fsi_slave_release; > + slave->link = link; > + slave->id = id; > + slave->size = FSI_SLAVE_SIZE_23b; > + > + dev_set_name(&slave->dev, "slave@%02x:%02x", link, id); > + rc = device_register(&slave->dev); > + if (rc < 0) { > + dev_warn(&master->dev, "failed to create slave device: %d\n", > + rc); > + put_device(&slave->dev); > + return rc; > + } > + > + /* todo: perform engine scan */ > > - return -ENODEV; > + return rc; > } > > /* FSI master support */ > -- > 1.8.2.2 >