Re: [RFC PATCH v1 14/14] usb:cdns3: Feature for changing role

[Roger Quadros]

On 08/11/18 13:51, Pawel Laszczak wrote:
>> On 03/11/18 19:51, Pawel Laszczak wrote:
>>> Patch adds feature that allow to change role from user space.
>>>
>>> Signed-off-by: Pawel Laszczak <pawell@xxxxxxxxxxx>
>>> ---
>>>  drivers/usb/cdns3/Makefile  |  2 +-
>>>  drivers/usb/cdns3/core.c    |  2 +
>>>  drivers/usb/cdns3/debugfs.c | 94 +++++++++++++++++++++++++++++++++++++
>>>  drivers/usb/cdns3/drd.h     |  3 ++
>>>  4 files changed, 100 insertions(+), 1 deletion(-)
>>>  create mode 100644 drivers/usb/cdns3/debugfs.c
>>>
>>> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
>>> index 34e60d03c4ec..08e6cdbebd46 100644
>>> --- a/drivers/usb/cdns3/Makefile
>>> +++ b/drivers/usb/cdns3/Makefile
>>> @@ -1,7 +1,7 @@
>>>  obj-$(CONFIG_USB_CDNS3)			+= cdns3.o
>>>  obj-$(CONFIG_USB_CDNS3_PCI_WRAP)	+= cdns3-pci.o
>>>
>>> -cdns3-y					:= core.o drd.o
>>> +cdns3-y					:= core.o drd.o debugfs.o
>>>  cdns3-$(CONFIG_USB_CDNS3_GADGET)	+= gadget.o ep0.o debug.o
>>>  cdns3-$(CONFIG_USB_CDNS3_HOST)		+= host.o
>>>  cdns3-pci-y		 		:= cdns3-pci-wrap.o
>>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>>> index 20ae9e76940e..4012f1007da9 100644
>>> --- a/drivers/usb/cdns3/core.c
>>> +++ b/drivers/usb/cdns3/core.c
>>> @@ -309,6 +309,7 @@ static int cdns3_probe(struct platform_device *pdev)
>>>  	if (ret)
>>>  		goto err4;
>>>
>>> +	cdns3_debugfs_init(cdns);
>>>  	device_set_wakeup_capable(dev, true);
>>>  	pm_runtime_set_active(dev);
>>>  	pm_runtime_enable(dev);
>>> @@ -346,6 +347,7 @@ static int cdns3_remove(struct platform_device *pdev)
>>>  	pm_runtime_get_sync(&pdev->dev);
>>>  	pm_runtime_disable(&pdev->dev);
>>>  	pm_runtime_put_noidle(&pdev->dev);
>>> +	cdns3_debugfs_exit(cdns);
>>>  	cdns3_remove_roles(cdns);
>>>  	usb_phy_shutdown(cdns->usbphy);
>>>
>>> diff --git a/drivers/usb/cdns3/debugfs.c b/drivers/usb/cdns3/debugfs.c
>>> new file mode 100644
>>> index 000000000000..d4871bc1a69d
>>> --- /dev/null
>>> +++ b/drivers/usb/cdns3/debugfs.c
>>> @@ -0,0 +1,94 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Cadence USBSS DRD Controller DebugFS filer.
>>> + *
>>> + * Copyright (C) 2018 Cadence.
>>> + *
>>> + * Author: Pawel Laszczak <pawell@xxxxxxxxxxx>
>>> + */
>>> +
>>> +#include <linux/types.h>
>>> +#include <linux/debugfs.h>
>>> +#include <linux/seq_file.h>
>>> +#include <linux/uaccess.h>
>>> +
>>> +#include "core.h"
>>> +#include "gadget.h"
>>> +
>>> +static int cdns3_mode_show(struct seq_file *s, void *unused)
>>> +{
>>> +	struct cdns3		*cdns = s->private;
>>> +
>>> +	switch (cdns->role) {
>>> +	case CDNS3_ROLE_HOST:
>>> +		seq_puts(s, "host\n");
>>> +		break;
>>> +	case CDNS3_ROLE_GADGET:
>>> +		seq_puts(s, "device\n");
>>> +		break;
>>> +	case CDNS3_ROLE_OTG:
>>> +	case CDNS3_ROLE_END:
>>> +		seq_puts(s, "otg\n");
>>> +		break;
>>> +	default:
>>> +		seq_puts(s, "UNKNOWN mode\n");
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int cdns3_mode_open(struct inode *inode, struct file *file)
>>> +{
>>> +	return single_open(file, cdns3_mode_show, inode->i_private);
>>> +}
>>> +
>>> +static ssize_t cdns3_mode_write(struct file *file,
>>> +				const char __user *ubuf,
>>> +				size_t count, loff_t *ppos)
>>> +{
>>> +	struct seq_file		*s = file->private_data;
>>> +	struct cdns3		*cdns = s->private;
>>> +	u32			mode = 0;
>>> +	char			buf[32];
>>> +
>>> +	if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
>>> +		return -EFAULT;
>>> +
>>> +	if (!strncmp(buf, "host", 4))
>>> +		mode = USB_DR_MODE_HOST;
>>> +
>>> +	if (!strncmp(buf, "device", 6))
>>> +		mode = USB_DR_MODE_PERIPHERAL;
>>> +
>>> +	if (!strncmp(buf, "otg", 3))
>>> +		mode = USB_DR_MODE_OTG;
>>> +
>>> +	cdns->desired_role = mode;
>>> +	queue_work(system_freezable_wq, &cdns->role_switch_wq);
>>
>> If we start with OTG mode and user says change mode to device will we still
>> switch to host based on ID pin change?
>>
>> If it does then this isn't working correctly.
>> We need to stop processing ID interrupts and keep the role static till
>> the user switches it back to otg.
> 
> Switching role form user space will limited driver only to selected mode. 
> Only for USB_DR_MODE_OTG driver should base on ID pin. 
> That's my intension.
> 

User space setting should override ID if it was OTG mode. At least this is how
it is for dwc3. That way it is useful for testing role swap.

cheers,
-roger

>>> +	return count;
>>> +}
>>> +
>>> +static const struct file_operations cdns3_mode_fops = {
>>> +	.open			= cdns3_mode_open,
>>> +	.write			= cdns3_mode_write,
>>> +	.read			= seq_read,
>>> +	.llseek			= seq_lseek,
>>> +	.release		= single_release,
>>> +};
>>> +
>>> +void cdns3_debugfs_init(struct cdns3 *cdns)
>>> +{
>>> +	struct dentry *root;
>>> +
>>> +	root = debugfs_create_dir(dev_name(cdns->dev), NULL);
>>> +	cdns->root = root;
>>> +	if (IS_ENABLED(CONFIG_USB_CDNS3_GADGET) &&
>>> +	    IS_ENABLED(CONFIG_USB_CDNS3_HOST))
>>> +		debugfs_create_file("mode", 0644, root, cdns,
>>> +				    &cdns3_mode_fops);
>>> +}
>>> +
>>> +void cdns3_debugfs_exit(struct cdns3 *cdns)
>>> +{
>>> +	debugfs_remove_recursive(cdns->root);
>>> +}
>>> diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
>>> index 85731f3b693c..5344290c76f2 100644
>>> --- a/drivers/usb/cdns3/drd.h
>>> +++ b/drivers/usb/cdns3/drd.h
>>> @@ -119,4 +119,7 @@ int cdns3_drd_probe(struct cdns3 *cdns);
>>>  void cdns3_set_hw_mode(struct cdns3 *cdns, u32 mode);
>>>  irqreturn_t cdns3_drd_irq(struct cdns3 *cdns);
>>>
>>> +void cdns3_debugfs_init(struct cdns3 *cdns);
>>> +void cdns3_debugfs_exit(struct cdns3 *cdns);
>>> +
>>>  #endif /* __LINUX_CDNS3_DRD */
>>>
>>
>> cheers,
>> -roger
>> --
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> 
> Thanks,
> Cheers,
> Pawel
> 

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki