[PATCH linux dev-4.7 v2 1/2] drivers: fsi: i2c fix probing

Christopher Bostic cbostic at linux.vnet.ibm.com
Sat Feb 11 03:59:12 AEDT 2017


Acked-by: Christopher Bostic <cbostic at linux.vnet.ibm.com>


On 2/10/17 10:49 AM, Eddie James wrote:
> From: "Edward A. James" <eajames at us.ibm.com>
>
> resolve the problems when probing up the i2c driver a second time.
>
> Signed-off-by: Edward A. James <eajames at us.ibm.com>
> ---
>   drivers/fsi/i2c/iic-fsi.c  | 125 +++------------------------------------------
>   drivers/fsi/i2c/iic-mstr.c |  42 +++------------
>   2 files changed, 14 insertions(+), 153 deletions(-)
>
> diff --git a/drivers/fsi/i2c/iic-fsi.c b/drivers/fsi/i2c/iic-fsi.c
> index 2c0aec9..6d5b045 100644
> --- a/drivers/fsi/i2c/iic-fsi.c
> +++ b/drivers/fsi/i2c/iic-fsi.c
> @@ -36,60 +36,12 @@
>   struct class* iic_fsi_class = 0;
>   dev_t iic_devnum_start = 0;
>
> -static const char iic_fsi_version[] = "3.0";
> +static const char iic_fsi_version[] = "3.1";
>
>   int iic_fsi_probe(struct device *dev);
>   int iic_fsi_remove(struct device *dev);
> -void iic_fsi_shutdown(struct device *dev);
>   int iic_fsi_suspend(struct device *dev);
>   int iic_fsi_resume(struct device *dev);
> -static void iic_eng_release(struct kobject* kobj);
> -
> -/* callback function for when the reference count for an engine reaches 0 */
> -static void iic_eng_release(struct kobject* kobj)
> -{
> -	iic_eng_t* eng = container_of(kobj, iic_eng_t, kobj);
> -	struct device* dev = eng->dev;
> -	unsigned long flags;
> -
> -	IENTER();
> -
> -	/*remove all busses associated with the engine */
> -	spin_lock_irqsave(&eng->lock, flags);
> -	while(eng->busses){
> -		iic_bus_t* temp = eng->busses;
> -		eng->busses = temp->next;
> -		iic_delete_bus(iic_fsi_class, temp);
> -	}
> -
> -	eng->enabled = 0x0ULL;
> -	spin_unlock_irqrestore(&eng->lock, flags);
> -
> -
> -	/* providing an arch specific cleanup routine is optional.
> -	 * if not specified, use the default.
> -	 */
> -	if (iic_eng_ops_is_vaild(eng->ops)) {
> -		if(eng->ops->cleanup_eng)
> -		{
> -			eng->ops->cleanup_eng(eng);
> -		}
> -		else
> -		{
> -			IDBGd(0, "free engine\n");
> -			kfree(eng);
> -		}
> -	}
> -
> -	dev_set_drvdata(dev, 0);
> -	kobject_put(&dev->kobj);
> -
> -	IEXIT(0);
> -}
> -
> -static struct kobj_type iic_eng_ktype = {
> -	.release = iic_eng_release,
> -};
>
>   int readb_wrap(iic_eng_t* eng, unsigned int addr, unsigned char *val,
>   	       iic_ffdc_t** ffdc)
> @@ -223,7 +175,7 @@ int iic_add_ports(iic_eng_t* eng, uint64_t ports)
>
>   	IENTER();
>
> -	IFLDi(3, "Adding ports[0x%08x%08x] to eng[0x%08x]",
> +	IFLDi(3, "Adding ports[0x%08x%08x] to eng[0x%08x]\n",
>   	      (uint32_t)(ports >> 32),
>   	      (uint32_t)ports,
>   	      eng->id);
> @@ -305,8 +257,6 @@ int iic_del_ports(iic_eng_t* eng, uint64_t ports)
>
>   	/* walk unordered SLL and delete bus if it is in the ports bit mask */
>   	spin_lock_irqsave(&eng->lock, flags);
> -	if(test_bit(IIC_ENG_REMOVED, &eng->flags))
> -		goto exit;
>   	p_abusp = &eng->busses;
>   	abusp = *p_abusp;
>   	while(abusp)
> @@ -316,7 +266,6 @@ int iic_del_ports(iic_eng_t* eng, uint64_t ports)
>   			/* found a match, remove it */
>   			*p_abusp = abusp->next;
>   			eng->enabled &= ~(0x1ULL << abusp->port);
> -			device_destroy(iic_fsi_class, abusp->devnum);
>   			iic_delete_bus(iic_fsi_class, abusp);
>   		}
>   		else
> @@ -327,12 +276,12 @@ int iic_del_ports(iic_eng_t* eng, uint64_t ports)
>   		abusp = *p_abusp;
>   	}
>
> -exit:
>   	spin_unlock_irqrestore(&eng->lock, flags);
>   	IEXIT(0);
>   	return 0;
>   }
>
> +#define IIC_FSI_PORTS	0xFFFULL
>   /*
>    * Called when an FSI IIC engine is plugged in.
>    * Causes creation of the /dev entry.
> @@ -361,12 +310,11 @@ int iic_fsi_probe(struct device *dev)
>   	iic_init_eng(eng);
>   	set_bit(IIC_ENG_BLOCK, &eng->flags); //block until resumed
>   	eng->id = 0x00F5112C;
> -	IFLDi(1, "PROBE    eng[%08x]", eng->id);
> +	IFLDi(1, "PROBE    eng[%08x]\n", eng->id);
>   	eng->ra = &fsi_reg_access;
>   	IFLDd(1, "vaddr=%#08lx\n", eng->base);
>   	eng->dev = dev;
>   	// The new kernel now requires 2 arguments
> -	kobject_init(&eng->kobj, &iic_eng_ktype); //ref count = 1
>   	eng->ops = iic_get_eng_ops(FSI_ENGID_I2C);
>   	if(!eng->ops)
>   	{
> @@ -382,7 +330,7 @@ int iic_fsi_probe(struct device *dev)
>
>   	IFLDd(1, "irq  = %d\n", eng->irq);
>
> -	new_ports = 0xFFFULL;
> +	new_ports = IIC_FSI_PORTS;
>   	set_bit(IIC_ENG_P8_Z8_CENTAUR, &eng->flags);
>
>
> @@ -394,21 +342,13 @@ int iic_fsi_probe(struct device *dev)
>   	dev_set_drvdata(dev, eng);
>   	eng->private_data = 0; //unused
>
> -
> -	/* set the callback function for when the eng ref count reaches 0 */
> -	kobject_get(&eng->dev->kobj);
> -
>   	iic_fsi_resume(dev);
>
>   error:
>   	if(rc)
>   	{
>   		IFLDi(1, "IIC: iic_fsi_probe failed: %d\n", rc);
> -		while(eng && eng->busses){
> -			iic_bus_t* temp = eng->busses;
> -			eng->busses = temp->next;
> -			iic_delete_bus(iic_fsi_class, temp);
> -		}
> +		iic_del_ports(eng, new_ports);
>   		if(eng)
>   		{
>   			kfree(eng);
> @@ -430,9 +370,7 @@ error:
>   int iic_fsi_remove(struct device* dev)
>   {
>   	int rc = 0;
> -	iic_bus_t* bus;
>   	iic_eng_t* eng = (iic_eng_t*)dev_get_drvdata(dev);
> -	unsigned long flags;
>
>   	IENTER();
>          
> @@ -453,62 +391,15 @@ int iic_fsi_remove(struct device* dev)
>   	IFLDi(1, "REMOVE   eng[%08x]\n", eng->id);
>
>   	/* Clean up device files immediately, don't wait for ref count */
> -	spin_lock_irqsave(&eng->lock, flags);
> -	bus = eng->busses;
> -	while(bus)
> -	{
> -		/* causes hot unplug event */
> -		device_destroy(iic_fsi_class, bus->devnum);
> -		bus = bus->next;
> -	}
> -	spin_unlock_irqrestore(&eng->lock, flags);
> -
> +	iic_del_ports(eng, IIC_FSI_PORTS);
>   	/* cleans up engine and bus structures if ref count is zero */
> -	kobject_put(&eng->kobj);
> +	kfree(eng);
>   	
>   error:
>   	IEXIT(0);
>   	return 0;
>   }
>
> -/* This function is called when a link is removed or the driver is unloaded.
> - * It's job is to quiesce and disable the hardware if possible and unregister
> - * interrupts. It always precedes the remove function.
> - *
> - * The device may be in the resumed or suspended state when this function is
> - * called.
> - *
> - * This function is no longer called for mcp5
> - */
> -void iic_fsi_shutdown(struct device *dev)
> -{
> -	int rc = 0;
> -	iic_eng_t* eng = (iic_eng_t*)dev_get_drvdata(dev);
> -
> -	IENTER();
> -	if(!eng || !eng->ops)
> -	{
> -		rc = -1;
> -		goto error;
> -	}
> -	IFLDi(1, "SHUTDOWN eng[%08x]\n", eng->id);
> -
> -	/* set ENG_REMOVED flag so that aborted operations have status
> -	 * set to ENOLINK (lost fsi link) instead of ENODEV (no lbus).
> -	 */
> -	set_bit(IIC_ENG_REMOVED, &eng->flags);
> -
> -	iic_fsi_suspend(dev);
> -	
> -error:
> -	if(rc)
> -	{
> -		IFLDe(1, "iic_fsi_shutdown failed: %d\n", rc);
> -	}
> -	IEXIT(0);
> -	return;
> -}
> -
>   /* This function is called when we loose ownership or are preparing to give
>    * up ownership of the local bus.  If we still own lbus, then we try to
>    * gracefully halt any pending transfer.  No hot unplug events are caused by
> diff --git a/drivers/fsi/i2c/iic-mstr.c b/drivers/fsi/i2c/iic-mstr.c
> index 0d97671..5501b22 100644
> --- a/drivers/fsi/i2c/iic-mstr.c
> +++ b/drivers/fsi/i2c/iic-mstr.c
> @@ -83,27 +83,7 @@ iic_opts_t iic_dflt_opts =
>   	}
>   };
>
> -static const char iic_mstr_version[] = "3.0";
> -
> -/* save off the default cdev type pointer so we can call the default cdev
> - * release function in our own bus release function
> - */
> -static struct kobj_type* cdev_dflt_type = 0;
> -struct kobj_type iic_bus_type;
> -
> -/* funtion called when cdev object (embedded in bus object) ref count
> - * reaches zero.  (prevents cdev memory from being freed to early)
> - */
> -void iic_bus_release(struct kobject* kobj)
> -{
> -	struct cdev *p = container_of(kobj, struct cdev, kobj);
> -	iic_bus_t* bus = container_of(p, iic_bus_t, cdev);
> -
> -	IFLDi(1, "deleting bus[%08lx]\n", bus->bus_id);
> -	if(cdev_dflt_type && cdev_dflt_type->release)
> -		cdev_dflt_type->release(kobj);
> -	kfree(bus);
> -}
> +static const char iic_mstr_version[] = "3.1";
>
>   int iic_open(struct inode* inode, struct file* filp);
>   int iic_release(struct inode* inode, struct file* filp);
> @@ -167,7 +147,6 @@ int iic_common_open(iic_client_t ** o_client, iic_bus_t * bus, int engine_num)
>   	client->tgid = current->tgid;
>   	sema_init(&client->sem, 1);
>   	init_waitqueue_head(&client->wait);
> -	kobject_get(&bus->eng->kobj);
>   	*o_client = client;
>
>   exit:
> @@ -262,7 +241,6 @@ int iic_common_release(iic_client_t * client)
>
>           client->bus = 0;
>           kfree(client);
> -        kobject_put(&bus->eng->kobj);
>
>           IEXIT(rc);
>           return rc;
> @@ -1170,6 +1148,7 @@ EXPORT_SYMBOL(iic_sideways_read);
>   ssize_t iic_read(struct file *filp, char __user *buf, size_t count,
>   		 loff_t *offset)
>   {
> +	int rc_copy;
>   	ssize_t rc = count;
>   	char *kbuf;
>   	iic_client_t *client = (iic_client_t*)filp->private_data;
> @@ -1203,7 +1182,7 @@ ssize_t iic_read(struct file *filp, char __user *buf, size_t count,
>   	if (rc < 0)
>   		goto free;
>
> -	rc = copy_to_user(buf, kbuf, count);
> +	rc_copy = copy_to_user(buf, kbuf, count);
>
>   free:
>   	kfree(kbuf);
> @@ -2148,23 +2127,11 @@ iic_bus_t*  iic_create_bus(struct class* classp, iic_eng_t* eng,
>   	bus->devnum = devnum;
>   	bus->i2c_hz = 400000;
>   	cdev_init(&bus->cdev, &iic_fops); // ref count = 1
> -	/* since cdev is embedded in our bus structure, override the cdev
> -	 * cleanup function with our own so that the bus object doesn't get
> -	 * freed until the cdev ref count goes to zero.
> -	 */
> -	if(!cdev_dflt_type)
> -	{
> -		cdev_dflt_type = bus->cdev.kobj.ktype;
> -		memcpy(&iic_bus_type, cdev_dflt_type, sizeof(iic_bus_type));
> -		iic_bus_type.release = iic_bus_release;
> -	}
> -	bus->cdev.kobj.ktype = &iic_bus_type;
>   	kobject_set_name(&bus->cdev.kobj, name);
>   	rc = cdev_add(&bus->cdev, devnum, 1);
>   	if(rc)
>   	{
>   		IFLDe(1, "cdev_add failed for bus %08lx\n", bus->bus_id);
> -		kobject_put(&bus->cdev.kobj);
>   		goto exit_cdev_add;
>   	}
>
> @@ -2186,6 +2153,7 @@ iic_bus_t*  iic_create_bus(struct class* classp, iic_eng_t* eng,
>   exit_class_add:
>   	cdev_del(&bus->cdev);
>   exit_cdev_add:
> +	kfree(bus);
>   	bus = 0;
>   exit:
>   	IEXIT((int)bus);
> @@ -2202,7 +2170,9 @@ void iic_delete_bus(struct class* classp, iic_bus_t* bus)
>   		goto exit;
>   	}
>   	IFLDi(1, "cleanup bus[%08lx]\n", bus->bus_id);
> +	device_destroy(classp, bus->devnum);
>   	cdev_del(&bus->cdev);
> +	kfree(bus);
>   exit:
>   	IEXIT(0);
>   	return;



More information about the openbmc mailing list