Re: [PATCH net-next 4/5] treewide: replace dev->trans_start update with helper
On 05/03/2016 04:33 PM, Florian Westphal wrote:
> Replace all trans_start updates with netif_trans_update helper.
> change was done via spatch:
>
> struct net_device *d;
> @@
> - d->trans_start = jiffies
> + netif_trans_update(d)
>
> Compile tested only.
>
> Cc: user-mode-linux-de...@lists.sourceforge.net
> Cc: linux-xte...@linux-xtensa.org
> Cc: linux1394-de...@lists.sourceforge.net
> Cc: linux-r...@vger.kernel.org
> Cc: net...@vger.kernel.org
> Cc: mpt-fusionlinux@broadcom.com
> Cc: linux-s...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> Cc: linux-par...@vger.kernel.org
> Cc: linux-o...@vger.kernel.org
> Cc: linux-h...@vger.kernel.org
> Cc: linux-usb@vger.kernel.org
> Cc: linux-wirel...@vger.kernel.org
> Cc: linux-s...@vger.kernel.org
> Cc: de...@driverdev.osuosl.org
> Cc: b.a.t.m@lists.open-mesh.org
> Cc: linux-blueto...@vger.kernel.org
> Signed-off-by: Florian Westphal
> ---
> Checkpatch complains about whitespace damage, but
> this extra whitespace already exists before this patch.
>
> drivers/net/can/mscan/mscan.c | 4 ++--
> drivers/net/can/usb/ems_usb.c | 4 ++--
> drivers/net/can/usb/esd_usb2.c | 4 ++--
> drivers/net/can/usb/peak_usb/pcan_usb_core.c | 4 ++--
> diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
> index e36b740..acb708f 100644
> --- a/drivers/net/can/mscan/mscan.c
> +++ b/drivers/net/can/mscan/mscan.c
> @@ -276,7 +276,7 @@ static netdev_tx_t mscan_start_xmit(struct sk_buff *skb,
> struct net_device *dev)
> out_8(®s->cantflg, 1 << buf_id);
>
> if (!test_bit(F_TX_PROGRESS, &priv->flags))
> - dev->trans_start = jiffies;
> + netif_trans_update(dev);
>
> list_add_tail(&priv->tx_queue[buf_id].list, &priv->tx_head);
>
> @@ -469,7 +469,7 @@ static irqreturn_t mscan_isr(int irq, void *dev_id)
> clear_bit(F_TX_PROGRESS, &priv->flags);
> priv->cur_pri = 0;
> } else {
> - dev->trans_start = jiffies;
> + netif_trans_update(dev);
> }
>
> if (!test_bit(F_TX_WAIT_ALL, &priv->flags))
> diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
> index 3400fd1..71f0e79 100644
> --- a/drivers/net/can/usb/ems_usb.c
> +++ b/drivers/net/can/usb/ems_usb.c
> @@ -521,7 +521,7 @@ static void ems_usb_write_bulk_callback(struct urb *urb)
> if (urb->status)
> netdev_info(netdev, "Tx URB aborted (%d)\n", urb->status);
>
> - netdev->trans_start = jiffies;
> + netif_trans_update(netdev);
>
> /* transmission complete interrupt */
> netdev->stats.tx_packets++;
> @@ -835,7 +835,7 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff
> *skb, struct net_device *ne
> stats->tx_dropped++;
> }
> } else {
> - netdev->trans_start = jiffies;
> + netif_trans_update(netdev);
>
> /* Slow down tx path */
> if (atomic_read(&dev->active_tx_urbs) >= MAX_TX_URBS ||
> diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
> index 113e64f..784a900 100644
> --- a/drivers/net/can/usb/esd_usb2.c
> +++ b/drivers/net/can/usb/esd_usb2.c
> @@ -480,7 +480,7 @@ static void esd_usb2_write_bulk_callback(struct urb *urb)
> if (urb->status)
> netdev_info(netdev, "Tx URB aborted (%d)\n", urb->status);
>
> - netdev->trans_start = jiffies;
> + netif_trans_update(netdev);
> }
>
> static ssize_t show_firmware(struct device *d,
> @@ -820,7 +820,7 @@ static netdev_tx_t esd_usb2_start_xmit(struct sk_buff
> *skb,
> goto releasebuf;
> }
>
> - netdev->trans_start = jiffies;
> + netif_trans_update(netdev);
>
> /*
>* Release our reference to this URB, the USB core will eventually free
> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> index 5a2e341..bfb91d8 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> @@ -274,7 +274,7 @@ static void peak_usb_write_bulk_callback(struct urb *urb)
> netdev->stats.tx_bytes += context->data_len;
>
> /* prevent tx timeout */
> - netdev->trans_start = jiffies;
> + netif_trans_update(netdev);
> break;
Re: [PATCH v6] can: kvaser_usb: Add support for Kvaser CAN/USB devices
On 11/21/2012 08:11 AM, Olivier Sobrie wrote:
> This driver provides support for several Kvaser CAN/USB devices.
> Such kind of devices supports up to three CAN network interfaces.
>
> It has been tested with a Kvaser USB Leaf Light (one network interface)
> connected to a pch_can interface.
> The firmware version of the Kvaser device was 2.5.205.
>
> List of Kvaser devices supported by the driver:
> - Kvaser Leaf Light
> - Kvaser Leaf Professional HS
> - Kvaser Leaf SemiPro HS
> - Kvaser Leaf Professional LS
> - Kvaser Leaf Professional SWC
> - Kvaser Leaf Professional LIN
> - Kvaser Leaf SemiPro LS
> - Kvaser Leaf SemiPro SWC
> - Kvaser Memorator II HS/HS
> - Kvaser USBcan Professional HS/HS
> - Kvaser Leaf Light GI
> - Kvaser Leaf Professional HS (OBD-II connector)
> - Kvaser Memorator Professional HS/LS
> - Kvaser Leaf Light "China"
> - Kvaser BlackBird SemiPro
> - Kvaser USBcan R
>
> Signed-off-by: Daniel Berglund
> Signed-off-by: Olivier Sobrie
> ---
[...]
> +static struct usb_device_id kvaser_usb_table[] = {
this can be const
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_DEVEL_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS |
> +KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS |
> +KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LS_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS |
> +KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_SWC_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS |
> +KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LIN_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS |
> +KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_LS_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS |
> +KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_SWC_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS |
> +KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_DEVEL_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS |
> +KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSHS_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS |
> +KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_UPRO_HSHS_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_GI_PRODUCT_ID) },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_OBDII_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS |
> +KVASER_HAS_SILENT_MODE },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSLS_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_CH_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_BLACKBIRD_SPRO_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_MERCURY_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_LEAF_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_CAN_R_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> + { }
> +};
[...]
> +static struct can_bittiming_const kvaser_usb_bittiming_const = {
this, too.
> + .name = "kvaser_usb",
> + .tseg1_min = KVASER_USB_TSEG1_MIN,
> + .tseg1_max = KVASER_USB_TSEG1_MAX,
> + .tseg2_min = KVASER_USB_TSEG2_MIN,
> + .tseg2_max = KVASER_USB_TSEG2_MAX,
> + .sjw_max = KVASER_USB_SJW_MAX,
> + .brp_min = KVASER_USB_BRP_MIN,
> + .brp_max = KVASER_USB_BRP_MAX,
> + .brp_inc = KVASER_USB_BRP_INC,
> +};
I'm adding the consts while applying the patch.
Thanks,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v6] can: kvaser_usb: Add support for Kvaser CAN/USB devices
On 11/22/2012 10:30 PM, Greg KH wrote: > On Thu, Nov 22, 2012 at 04:01:49PM +0100, Olivier Sobrie wrote: >> Hi linux-usb folks, >> >> Is there someone who can help me to fix the following errors? >> >> smatch warnings: >> >> + drivers/net/can/usb/kvaser_usb.c:431 kvaser_usb_send_simple_msg() error: >> doing >> +dma on the stack ((null)) >> + drivers/net/can/usb/kvaser_usb.c:1073 kvaser_usb_set_opt_mode() error: >> doing >> +dma on the stack ((null)) >> + drivers/net/can/usb/kvaser_usb.c:1174 kvaser_usb_flush_queue() error: doing >> +dma on the stack ((null)) >> + drivers/net/can/usb/kvaser_usb.c:1384 kvaser_usb_set_bittiming() error: >> doing >> +dma on the stack ((null)) >> >> I assume it's due to the buffer I pass to the function usb_bulk_msg() >> which is on the stack and can't be. >> Do I just have to kmalloc a buffer and give it to the usb_bulk_msg() >> function? That's what I understood by reading >> "Documentation/DMA-API-HOWTO.txt" section "What memory is DMA'able?"... >> and from commit >> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=32ec4576c3fb37316b1d11a04b220527822f3f0d > > Yes, that is all that is needed. Thanks Greg. Olivier, you can post an incremental patch, I'll squash it before sending the patches upstream. regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v3 3/7] usb: chipidea: usbmisc: fix a potential race condition
On 11/23/2012 06:36 AM, Peter Chen wrote: > On Wed, Nov 21, 2012 at 03:06:29PM +0100, Michael Grzeschik wrote: >> From: Marc Kleine-Budde >> >> This fixes a potential race condition where the ci13xxx_imx glue code >> could be fast enough to call one of the usbmisc_ops before he got a >> valid value on the static usbmisc pointer. To fix that we first set >> usbmisc, then call usbmisc_set_ops(). > > usbmisc is subsys_initcall, and cil13xxx_imx is module_init. Any > potential situation that the ci13xxx_imx's probe is ran before the > usbmisc's probe is completed? Besides, there is usbmisc_ops value > check at the beginning of cil13xxx_imx's probe. It's bad practice to rely your code on some external ordering mechanism, even more if the correct solution is so simple. And as Sascha pointed out everything might be build as modules and loaded individually. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v3 6/7] usb: chipidea: usbmisc: add mx53 support
On 11/23/2012 07:53 AM, Peter Chen wrote: > On Wed, Nov 21, 2012 at 03:06:32PM +0100, Michael Grzeschik wrote: >> This adds mx53 as the next user of the usbmisc driver and makes it >> possible to disable the overcurrent-detection of the internal phy. >> >> Signed-off-by: Michael Grzeschik >> Signed-off-by: Marc Kleine-Budde >> --- >> Changes since v2: >> * added defines for register offsets and bitmasks >> * fixed disable_oc option for all ports >> >> drivers/usb/chipidea/usbmisc_imx.c | 51 >> >> 1 file changed, 51 insertions(+) >> >> diff --git a/drivers/usb/chipidea/usbmisc_imx.c >> b/drivers/usb/chipidea/usbmisc_imx.c >> index e090e0f..b1811df 100644 >> --- a/drivers/usb/chipidea/usbmisc_imx.c >> +++ b/drivers/usb/chipidea/usbmisc_imx.c >> @@ -19,6 +19,13 @@ >> >> #define USB_DEV_MAX 4 >> >> +#define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08; >> +#define MX53_USB_UH2_CTRL_OFFSET0x14; >> +#define MX53_USB_UH3_CTRL_OFFSET0x18; >> +#define MX53_BM_OVER_CUR_DIS_H1 BIT(5) >> +#define MX53_BM_OVER_CUR_DIS_OTGBIT(8) >> +#define MX53_BM_OVER_CUR_DIS_UHxBIT(30) >> + >> #define MX6_BM_OVER_CUR_DIS BIT(7) > > As every SoC platform's non-core register mapping is different, > it is better to use .h to put register bit definition AFAIK in the kernel we don't make extra .h files any more, if there is only a single user of the contents. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH] kvaser_usb: fix "dma on the stack" errors
On 11/23/2012 02:40 PM, Olivier Sobrie wrote:
> On Fri, Nov 23, 2012 at 02:30:28PM +0100, Olivier Sobrie wrote:
>> The dma buffer given to usb_bulk_msg() must be allocated and not on
>> the stack.
>> See Documentation/DMA-API-HOWTO.txt section "What memory is DMA'able?"
>>
>> Signed-off-by: Olivier Sobrie
>> ---
>> Here is the incremental patch.
>> Thank you Greg !
>>
>> Olivier
>>
>> drivers/net/can/usb/kvaser_usb.c | 110
>> --
>> 1 file changed, 69 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/net/can/usb/kvaser_usb.c
>> b/drivers/net/can/usb/kvaser_usb.c
>> index 8807bf8..7ac6e82 100644
>> --- a/drivers/net/can/usb/kvaser_usb.c
>> +++ b/drivers/net/can/usb/kvaser_usb.c
>> @@ -421,14 +421,21 @@ end:
>> static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev,
>>u8 msg_id, int channel)
>> {
>> -struct kvaser_msg msg = {
>> -.len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple),
>> -.id = msg_id,
>> -.u.simple.channel = channel,
>> -.u.simple.tid = 0xff,
>> -};
>> -
>> -return kvaser_usb_send_msg(dev, &msg);
>> +struct kvaser_msg *msg;
>> +int rc;
>> +
>> +msg = kmalloc(sizeof(*msg), GFP_KERNEL);
>> + if (!msg)
>> +return -ENOMEM;
>> +
>
> Doh! I removed by mistake the line "msg->id = msg_id"... grr
Please send a v2 version of this patch with this problem fixed.
MarcMarc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v2] kvaser_usb: fix "dma on the stack" errors
On 11/23/2012 02:54 PM, Olivier Sobrie wrote: > The dma buffer given to usb_bulk_msg() must be allocated and not on > the stack. > See Documentation/DMA-API-HOWTO.txt section "What memory is DMA'able?" > > Signed-off-by: Olivier Sobrie Thanks, I've squashed it into the original patch and pushed to linux-can-next/for-davem Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v2] usb_8dev: Add support for USB2CAN interface from 8 devices
> +.ndo_open = usb_8dev_open,
> +.ndo_stop = usb_8dev_close,
> +.ndo_start_xmit = usb_8dev_start_xmit,
> +};
> +
> +static struct can_bittiming_const usb_8dev_bittiming_const = {
const
> +.name = "usb_8dev",
> +.tseg1_min = USB_8DEV_TSEG1_MIN,
> +.tseg1_max = USB_8DEV_TSEG1_MAX,
> +.tseg2_min = USB_8DEV_TSEG2_MIN,
> +.tseg2_max = USB_8DEV_TSEG2_MAX,
> +.sjw_max = USB_8DEV_SJW_MAX,
> +.brp_min = USB_8DEV_BRP_MIN,
> +.brp_max = USB_8DEV_BRP_MAX,
> +.brp_inc = USB_8DEV_BRP_INC,
> +};
> +
> +/*
> + * Probe USB device
> + *
> + * Check device and firmware.
> + * Set supported modes and bittiming constants.
> + * Allocate some memory.
> + */
> +static int usb_8dev_probe(struct usb_interface *intf,
> + const struct usb_device_id *id)
> +{
> +struct net_device *netdev;
> +struct usb_8dev *dev;
> +int i, err = -ENOMEM;
> +u32 version;
> +char buf[18];
> +struct usb_device *usbdev = interface_to_usbdev(intf);
> +
> +/* product id looks strange, better we also check iProdukt string */
> +if (usb_string(usbdev, usbdev->descriptor.iProduct, buf,
> + sizeof(buf)) > 0 && strcmp(buf, "USB2CAN converter")) {
> +dev_info(&usbdev->dev, "ignoring: not an USB2CAN converter\n");
> +return -ENODEV;
> +}
> +
> +netdev = alloc_candev(sizeof(struct usb_8dev), MAX_TX_URBS);
> +if (!netdev) {
> +dev_err(&intf->dev, "Couldn't alloc candev\n");
> +return -ENOMEM;
> +}
> +
> +dev = netdev_priv(netdev);
> +
> +dev->udev = usbdev;
> +dev->netdev = netdev;
> +
> +dev->can.state = CAN_STATE_STOPPED;
> +dev->can.clock.freq = USB_8DEV_ABP_CLOCK;
> +dev->can.bittiming_const = &usb_8dev_bittiming_const;
> +dev->can.do_set_bittiming = usb_8dev_set_bittiming;
> +dev->can.do_set_mode = usb_8dev_set_mode;
> +dev->can.do_get_berr_counter = usb_8dev_get_berr_counter;
> +dev->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> + CAN_CTRLMODE_LISTENONLY |
> + CAN_CTRLMODE_ONE_SHOT;
> +
> +netdev->netdev_ops = &usb_8dev_netdev_ops;
> +
> +netdev->flags |= IFF_ECHO; /* we support local echo */
> +
> +init_usb_anchor(&dev->rx_submitted);
> +
> +init_usb_anchor(&dev->tx_submitted);
> +atomic_set(&dev->active_tx_urbs, 0);
> +
> +for (i = 0; i < MAX_TX_URBS; i++)
> +dev->tx_contexts[i].echo_index = MAX_TX_URBS;
> +
> +dev->cmd_msg_buffer = kzalloc(sizeof(struct usb_8dev_cmd_msg),
> + GFP_KERNEL);
> +if (!dev->cmd_msg_buffer) {
> +dev_err(&intf->dev, "Couldn't alloc Tx buffer\n");
> +goto cleanup_candev;
> +}
> +
> +usb_set_intfdata(intf, dev);
> +
> +SET_NETDEV_DEV(netdev, &intf->dev);
> +
> +mutex_init(&dev->usb_8dev_cmd_lock);
> +
> +err = usb_8dev_cmd_version(dev, &version);
> +if (err) {
> +dev_err(netdev->dev.parent, "can't get firmware version\n");
> +goto cleanup_cmd_msg_buffer;
> +} else {
> +dev_info(netdev->dev.parent,
> + "firmware: %d.%d, hardware: %d.%d\n",
> + (version>>24) & 0xff, (version>>16) & 0xff,
> + (version>>8) & 0xff, version & 0xff);
> +}
> +
> +err = register_candev(netdev);
> +if (err) {
> +dev_err(netdev->dev.parent,
> +"couldn't register CAN device: %d\n", err);
> +goto cleanup_cmd_msg_buffer;
> +}
> +
> +if (device_create_file(&intf->dev, &dev_attr_firmware))
> +dev_err(&intf->dev,
> +"Couldn't create device file for firmware\n");
> +
> +if (device_create_file(&intf->dev, &dev_attr_hardware))
> +dev_err(&intf->dev,
> +"Couldn't create device file for hardware\n");
> +
> +/* let the user know what node this device is now attached to */
> +dev_info(netdev->dev.parent, "device registered as %s\n",
> netdev->name);
> +return 0;
> +
> +cleanup_cmd_msg_buffer:
> +kfree(dev->cmd_msg_buffer);
> +
> +cleanup_candev:
> +free_candev(netdev);
> +
> +return err;
> +
> +}
> +
> +/* Called by the usb core when driver is unloaded or device is removed */
> +static void usb_8dev_disconnect(struct usb_interface *intf)
> +{
> +struct usb_8dev *dev = usb_get_intfdata(intf);
> +
> +device_remove_file(&intf->dev, &dev_attr_firmware);
> +device_remove_file(&intf->dev, &dev_attr_hardware);
> +
> +usb_set_intfdata(intf, NULL);
> +
> +if (dev) {
> +dev_info(&intf->dev, "disconnect %s\n", dev->netdev->name);
> +
> +unregister_netdev(dev->netdev);
> +free_candev(dev->netdev);
> +
> +unlink_all_urbs(dev);
> +}
> +
> +}
> +
> +static struct usb_driver usb_8dev_driver = {
> +.name ="usb_8dev",
> +.probe =usb_8dev_probe,
> +.disconnect =usb_8dev_disconnect,
> +.id_table =usb_8dev_table,
> +};
> +
> +module_usb_driver(usb_8dev_driver);
> +
> +MODULE_AUTHOR("Bernd Krumboeck ");
> +MODULE_DESCRIPTION("CAN driver for UAB 8 devices USB2CAN interfaces");
> +MODULE_LICENSE("GPL v2");
> +
Please remove trailing empty line
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v2] usb_8dev: Add support for USB2CAN interface from 8 devices
On 12/03/2012 09:42 PM, krumbo...@universalnet.at wrote:
> Hi Marc!
>
>
>>> +/* Send open command to device */
>>> +static int usb_8dev_cmd_open(struct usb_8dev *dev)
>>> +{
>>> +struct can_bittiming *bt = &dev->can.bittiming;
>>> +struct usb_8dev_cmd_msg outmsg;
>>> +struct usb_8dev_cmd_msg inmsg;
>>> +u32 flags = 0;
>>> +u32 beflags;
>>> +u16 bebrp;
>>> +u32 ctrlmode = dev->can.ctrlmode;
>>> +
>>> +if (ctrlmode & CAN_CTRLMODE_LOOPBACK)
>>> +flags |= USB_8DEV_LOOPBACK;
>>> +if (ctrlmode & CAN_CTRLMODE_LISTENONLY)
>>> +flags |= USB_8DEV_SILENT;
>>> +if (ctrlmode & CAN_CTRLMODE_ONE_SHOT)
>>> +flags |= USB_8DEV_DISABLE_AUTO_RESTRANS;
>>> +
>>> +flags |= USB_8DEV_STATUS_FRAME;
>>> +
>>> +memset(&outmsg, 0, sizeof(struct usb_8dev_cmd_msg));
>>> +outmsg.command = USB_8DEV_OPEN;
>>> +outmsg.opt1 = USB_8DEV_BAUD_MANUAL;
>>> +outmsg.data[0] = (bt->prop_seg + bt->phase_seg1);
>>> +outmsg.data[1] = bt->phase_seg2;
>>> +outmsg.data[2] = bt->sjw;
>>
>> But you should not use usb_bulk_msg() to send data which is on the
>> stack, please use you already allocated memory in priv or kmalloc
>> something here.
>
> The function usb_8dev_send_cmd copies the data to dev->cmd_msg_buffer
> (allocated with kzalloc, GFP_KERNEL).
Okay, then you might want to set outmsg via C99 initializers.
>>> +
>>> +/* BRP */
>>> +bebrp = cpu_to_be16((u16) bt->brp);
>>> +memcpy(&outmsg.data[3], &bebrp, sizeof(bebrp));
>>
>> Are you sure about the endianess? Some data types are BE some are LE?
>
> Where is LE used?
Opps, yeah right. only BE.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v2] usb_8dev: Add support for USB2CAN interface from 8 devices
On 12/03/2012 09:15 PM, Wolfgang Grandegger wrote: > On 12/03/2012 09:32 PM, krumbo...@universalnet.at wrote: >> Hi Wolfgang! >> >>> >>> And please also drop the remaining sysfs files for firmware and >>> hardware. I thinks it's enough that the versions are printed when the >>> device is probed. >> >> Systemadministrators often use versions for configuration scripts. >> If you insist I will remove. > > Marc? Are these values exported via lsusb -v (e.g. iSerial)? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v3] usb_8dev: Add support for USB2CAN interface from 8 devices
tseg1_max = USB_8DEV_TSEG1_MAX,
> +.tseg2_min = USB_8DEV_TSEG2_MIN,
> +.tseg2_max = USB_8DEV_TSEG2_MAX,
> +.sjw_max = USB_8DEV_SJW_MAX,
> +.brp_min = USB_8DEV_BRP_MIN,
> +.brp_max = USB_8DEV_BRP_MAX,
> +.brp_inc = USB_8DEV_BRP_INC,
> +};
> +
> +/*
> + * Probe USB device
> + *
> + * Check device and firmware.
> + * Set supported modes and bittiming constants.
> + * Allocate some memory.
> + */
> +static int usb_8dev_probe(struct usb_interface *intf,
> + const struct usb_device_id *id)
> +{
> +struct net_device *netdev;
> +struct usb_8dev *dev;
> +int i, err = -ENOMEM;
> +u32 version;
> +char buf[18];
where does this 18 come from?
> +struct usb_device *usbdev = interface_to_usbdev(intf);
> +
> +/* product id looks strange, better we also check iProdukt string */
iProduct?
> +if (usb_string(usbdev, usbdev->descriptor.iProduct, buf,
> + sizeof(buf)) > 0 && strcmp(buf, "USB2CAN converter")) {
> +dev_info(&usbdev->dev, "ignoring: not an USB2CAN converter\n");
> +return -ENODEV;
> +}
> +
> +netdev = alloc_candev(sizeof(struct usb_8dev), MAX_TX_URBS);
> +if (!netdev) {
> +dev_err(&intf->dev, "Couldn't alloc candev\n");
> +return -ENOMEM;
> +}
> +
> +dev = netdev_priv(netdev);
> +
> +dev->udev = usbdev;
> +dev->netdev = netdev;
> +
> +dev->can.state = CAN_STATE_STOPPED;
> +dev->can.clock.freq = USB_8DEV_ABP_CLOCK;
> +dev->can.bittiming_const = &usb_8dev_bittiming_const;
> +dev->can.do_set_mode = usb_8dev_set_mode;
> +dev->can.do_get_berr_counter = usb_8dev_get_berr_counter;
> +dev->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> + CAN_CTRLMODE_LISTENONLY |
> + CAN_CTRLMODE_ONE_SHOT;
Have you actually tested one shot? What happens if a can frame cannot be
transmitted?
> +
> +netdev->netdev_ops = &usb_8dev_netdev_ops;
> +
> +netdev->flags |= IFF_ECHO; /* we support local echo */
> +
> +init_usb_anchor(&dev->rx_submitted);
> +
> +init_usb_anchor(&dev->tx_submitted);
> +atomic_set(&dev->active_tx_urbs, 0);
> +
> +for (i = 0; i < MAX_TX_URBS; i++)
> +dev->tx_contexts[i].echo_index = MAX_TX_URBS;
> +
> +dev->cmd_msg_buffer = kzalloc(sizeof(struct usb_8dev_cmd_msg),
> + GFP_KERNEL);
> +if (!dev->cmd_msg_buffer) {
> +netdev_err(netdev, "Couldn't alloc Tx buffer\n");
> +goto cleanup_candev;
> +}
> +
> +usb_set_intfdata(intf, dev);
> +
> +SET_NETDEV_DEV(netdev, &intf->dev);
> +
> +mutex_init(&dev->usb_8dev_cmd_lock);
> +
> +err = usb_8dev_cmd_version(dev, &version);
> +if (err) {
> +netdev_err(netdev, "can't get firmware version\n");
> +goto cleanup_cmd_msg_buffer;
> +} else {
> +netdev_info(netdev,
> + "firmware: %d.%d, hardware: %d.%d\n",
> + (version>>24) & 0xff, (version>>16) & 0xff,
> + (version>>8) & 0xff, version & 0xff);
> +}
> +
> +err = register_candev(netdev);
> +if (err) {
> +netdev_err(netdev,
> +"couldn't register CAN device: %d\n", err);
> +goto cleanup_cmd_msg_buffer;
> +}
> +
> +if (device_create_file(&intf->dev, &dev_attr_firmware))
> +netdev_err(netdev,
> +"Couldn't create device file for firmware\n");
> +
> +if (device_create_file(&intf->dev, &dev_attr_hardware))
> +netdev_err(netdev,
> +"Couldn't create device file for hardware\n");
> +
> +/* let the user know what node this device is now attached to */
> +netdev_info(netdev, "device registered as %s\n", netdev->name);
netdev_info should already print netdev->name
> +return 0;
> +
> +cleanup_cmd_msg_buffer:
> +kfree(dev->cmd_msg_buffer);
> +
> +cleanup_candev:
> +free_candev(netdev);
> +
> +return err;
> +
> +}
> +
> +/* Called by the usb core when driver is unloaded or device is removed */
> +static void usb_8dev_disconnect(struct usb_interface *intf)
> +{
> +struct usb_8dev *dev = usb_get_intfdata(intf);
> +
> +device_remove_file(&intf->dev, &dev_attr_firmware);
> +device_remove_file(&intf->dev, &dev_attr_hardware);
> +
> +usb_set_intfdata(intf, NULL);
> +
> +if (dev) {
> +netdev_info(dev->netdev, "disconnect %s\n", dev->netdev->name);
> +
> +unregister_netdev(dev->netdev);
> +free_candev(dev->netdev);
> +
> +unlink_all_urbs(dev);
> +}
> +
> +}
> +
> +static struct usb_driver usb_8dev_driver = {
> +.name ="usb_8dev",
> +.probe =usb_8dev_probe,
> +.disconnect =usb_8dev_disconnect,
> +.id_table =usb_8dev_table,
> +};
> +
> +module_usb_driver(usb_8dev_driver);
> +
> +MODULE_AUTHOR("Bernd Krumboeck ");
> +MODULE_DESCRIPTION("CAN driver for 8 devices USB2CAN interfaces");
> +MODULE_LICENSE("GPL v2");
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v4] usb_8dev: Add support for USB2CAN interface from 8 devices
On 12/04/2012 11:43 PM, Bernd Krumboeck wrote: > Add device driver for USB2CAN interface from "8 devices" > (http://www.8devices.com). > > Signed-off-by: Bernd Krumboeck Please use git send-email. A changelog would be nice, in the form of: changes since v4: - done this, deleted that If you post v5, changes since v4 are sufficient. > --- > drivers/net/can/usb/Kconfig|6 + > drivers/net/can/usb/Makefile |1 + > drivers/net/can/usb/usb_8dev.c | 1089 > > 3 files changed, 1096 insertions(+) > create mode 100644 drivers/net/can/usb/usb_8dev.c > > diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig > index a4e4bee..2162233 100644 > --- a/drivers/net/can/usb/Kconfig > +++ b/drivers/net/can/usb/Kconfig > @@ -48,4 +48,10 @@ config CAN_PEAK_USB >This driver supports the PCAN-USB and PCAN-USB Pro adapters >from PEAK-System Technik (http://www.peak-system.com). > > +config CAN_8DEV_USB > +tristate "8 devices USB2CAN interface" > +---help--- > + This driver supports the USB2CAN interface > + from 8 devices (http://www.8devices.com). > + > endmenu > diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile > index 80a2ee4..becef46 100644 > --- a/drivers/net/can/usb/Makefile > +++ b/drivers/net/can/usb/Makefile > @@ -6,5 +6,6 @@ obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o > obj-$(CONFIG_CAN_ESD_USB2) += esd_usb2.o > obj-$(CONFIG_CAN_KVASER_USB) += kvaser_usb.o > obj-$(CONFIG_CAN_PEAK_USB) += peak_usb/ > +obj-$(CONFIG_CAN_8DEV_USB) += usb_8dev.o > > ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG > diff --git a/drivers/net/can/usb/usb_8dev.c > b/drivers/net/can/usb/usb_8dev.c > new file mode 100644 > index 000..b3a7eee > --- /dev/null > +++ b/drivers/net/can/usb/usb_8dev.c > @@ -0,0 +1,1089 @@ > +/* > + * CAN driver for "8 devices" USB2CAN converter > + * > + * Copyright (C) 2012 Bernd Krumboeck (krumbo...@universalnet.at) > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published > + * by the Free Software Foundation; version 2 of the License. > + * > + * 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, write to the Free Software Foundation, Inc., > + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. you can remove the address here. > + * > + * This driver is inspired by the 3.2.0 version of > drivers/net/can/usb/ems_usb.c > + * and drivers/net/can/usb/esd_usb2.c > + * > + * Many thanks to Gerhard Bertelsmann (i...@gerhard-bertelsmann.de) > + * for testing and fixing this driver. Also many thanks to "8 devices", > + * who were very cooperative and answered my questions. > + */ Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v4] usb_8dev: Add support for USB2CAN interface from 8 devices
On 12/04/2012 11:11 PM, Marc Kleine-Budde wrote: > On 12/04/2012 11:43 PM, Bernd Krumboeck wrote: btw: your clock is wrong by about 1h :) Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v4] usb_8dev: Add support for USB2CAN interface from 8 devices
On 12/05/2012 04:49 PM, Oliver Hartkopp wrote: > On 05.12.2012 11:13, Wolfgang Grandegger wrote: > >>> +outmsg.command = USB_8DEV_OPEN; >>> +outmsg.opt1 = USB_8DEV_BAUD_MANUAL; >>> +outmsg.data[0] = (bt->prop_seg + bt->phase_seg1); >> >> Minor issue. Brackets not needed. >> >>> +outmsg.data[1] = bt->phase_seg2; >>> +outmsg.data[2] = bt->sjw; >>> + > > That's correct from a compilers point of view. > But in this case i would preserve the [0] as it is better to read in > conjunction with the two lines below ( ..[1] ..[2] ) > > The semantic has to be understood by humans ;-) I think Wolfgang was talking about the round bracket not the square bracket. :D Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v4] usb_8dev: Add support for USB2CAN interface from 8 devices
On 12/04/2012 11:43 PM, Bernd Krumboeck wrote:
> Add device driver for USB2CAN interface from "8 devices"
> (http://www.8devices.com).
[...]
> +/* Send data to device */
> +static netdev_tx_t usb_8dev_start_xmit(struct sk_buff *skb,
> + struct net_device *netdev)
> +{
> +struct usb_8dev *dev = netdev_priv(netdev);
I just noticed, it's unusual to name you private data pointer "dev", I
suggest to name it priv. Maybe you can rename your private data struct
to: "struct usb_8dev_priv".
> +struct net_device_stats *stats = &netdev->stats;
> +struct can_frame *cf = (struct can_frame *) skb->data;
> +struct usb_8dev_tx_msg *msg;
> +struct urb *urb;
> +struct usb_8dev_tx_urb_context *context = NULL;
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v3] usb_8dev: Add support for USB2CAN interface from 8 devices
On 12/05/2012 06:36 PM, Bernd Krumboeck wrote:
> Hi Marc!
>
>
>>> +default:
>>> +netdev_info(netdev, "Rx URB aborted (%d)\n",
>>> + urb->status);
>>> +goto resubmit_urb;
>>> +}
>>> +
>>> +while (pos < urb->actual_length) {
>>> +struct usb_8dev_rx_msg *msg;
>>> +
>>> +if (pos + sizeof(struct usb_8dev_rx_msg) >
>>> urb->actual_length) {
>>> +netdev_err(dev->netdev, "format error\n");
>>> +break;
>>
>> is resubmitting the urb the correct way to handle this problem?
>
> Suggestions? (maybe CAN_ERR_CRTL_UNSPEC ??)
It's not an error on the CAN protocol level, but the USB communication
is broken. I just had a look at the kvaser usb driver, it's doing a
resubmit, too. So it seems to be okay.
>
>>> +
>>> +stats->tx_dropped++;
>>> +}
>>> +} else {
>>> +/* Slow down tx path */
>>> +if (atomic_read(&dev->active_tx_urbs) >= MAX_TX_URBS ||
>>> +dev->free_slots < 5) {
>>
>> where's the 5 coming from?
>>
>
> From ems_usb driver.
H, is the variable free_slots used?
>
>>> +netif_stop_queue(netdev);
>>> +}
>>> +}
>>> +
>>> +/*
>>> + * Release our reference to this URB, the USB core will eventually
>>> free
>>> + * it entirely.
>>> + */
>>> +usb_free_urb(urb);
>>> +
>>> +return NETDEV_TX_OK;
>>> +
>>> +nomem:
>>> +dev_kfree_skb(skb);
>>> +stats->tx_dropped++;
>>> +
>>> +return NETDEV_TX_OK;
>>> +}
>>> +
>>> +static int usb_8dev_get_berr_counter(const struct net_device *netdev,
>>> + struct can_berr_counter *bec)
>>> +{
>>> +struct usb_8dev *dev = netdev_priv(netdev);
>>> +
>>> +bec->txerr = dev->bec.txerr;
>>> +bec->rxerr = dev->bec.rxerr;
>>> +
>>> +return 0;
>>> +}
>>> +
>>> +/* Start USB device */
>>> +static int usb_8dev_start(struct usb_8dev *dev)
>>> +{
>>> +struct net_device *netdev = dev->netdev;
>>> +int err, i;
>>> +
>>> +dev->free_slots = 15; /* initial size */
>>
>> there does the 15 come from?
>
> ditto
dito :)
>
>>> + * Check device and firmware.
>>> + * Set supported modes and bittiming constants.
>>> + * Allocate some memory.
>>> + */
>>> +static int usb_8dev_probe(struct usb_interface *intf,
>>> + const struct usb_device_id *id)
>>> +{
>>> +struct net_device *netdev;
>>> +struct usb_8dev *dev;
>>> +int i, err = -ENOMEM;
>>> +u32 version;
>>> +char buf[18];
>>
>> where does this 18 come from?
>
> String "USB2CAN converter" + trailing 0.
okay
>
>>
>>> +struct usb_device *usbdev = interface_to_usbdev(intf);
>>> +
>>> +/* product id looks strange, better we also check iProdukt
>>> string */
>>
>> iProduct?
I mean typo iProdukt vs iProduct.
>
> Check if usbdev->descriptor.iProduct == "USB2CAN converter".
>
>>> +if (usb_string(usbdev, usbdev->descriptor.iProduct, buf,
>>> + sizeof(buf)) > 0 && strcmp(buf, "USB2CAN converter")) {
>>> +dev_info(&usbdev->dev, "ignoring: not an USB2CAN converter\n");
>>> +return -ENODEV;
>>> +}
>>> +
>>> +netdev = alloc_candev(sizeof(struct usb_8dev), MAX_TX_URBS);
>>> +if (!netdev) {
>>> +dev_err(&intf->dev, "Couldn't alloc candev\n");
>>> +return -ENOMEM;
>>> +}
>>> +
>>> +dev = netdev_priv(netdev);
>>> +
>>> +dev->udev = usbdev;
>>> +dev->netdev = netdev;
>>> +
>>> +dev->can.state = CAN_STATE_STOPPED;
>>> +dev->can.clock.freq = USB_8DEV_ABP_CLOCK;
>>> +dev->can.bittiming_const = &usb_8dev_bittiming_const;
>>> +dev->can.do_set_mode = usb_8dev_set_mode;
>>> +dev->can.do_get_berr_counter = usb_8dev_get_berr_counter;
>>> +dev->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
>>> + CAN_CTRLMODE_LISTENONLY |
>>> + CAN_CTRLMODE_ONE_SHOT;
>>
>> Have you actually tested one shot? What happens if a can frame cannot be
>> transmitted?
>>
>
> Not really. Can someone explain what one-shot exactly means and what is
> the correct behavior.
> I've only tested without any other device connected. I got the same
> errors like in normal mode.
Can has a built in collision avoidance protocol. If a collision is about
to happen the sender with the higher CAN id will back of and try again
later. In one shot mode it will not try again. The question is, how
behaves the dongle firmware, as you have to free your tx message somehow.
To test, simply send with one station with canid 1, the other one in
one-shot-mode with canid 0x7ff.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v6] usb_8dev: Add support for USB2CAN interface from 8 devices
", err);
> +
> + return err;
> +}
> +
> +/* Open USB device */
> +static int usb_8dev_open(struct net_device *netdev)
> +{
> + struct usb_8dev_priv *priv = netdev_priv(netdev);
> + int err;
> +
> + /* common open */
> + err = open_candev(netdev);
> + if (err)
> + return err;
> +
> + /* finally start device */
> + err = usb_8dev_start(priv);
> + if (err) {
> + if (err == -ENODEV)
> + netif_device_detach(priv->netdev);
> +
> + netdev_warn(netdev, "couldn't start device: %d\n",
> + err);
> +
> + close_candev(netdev);
> +
> + return err;
> + }
> +
> + netif_start_queue(netdev);
> +
> + return 0;
> +}
> +
> +static void unlink_all_urbs(struct usb_8dev_priv *priv)
> +{
> + int i;
> +
> + usb_kill_anchored_urbs(&priv->rx_submitted);
> +
> + usb_kill_anchored_urbs(&priv->tx_submitted);
> + atomic_set(&priv->active_tx_urbs, 0);
> +
> + for (i = 0; i < MAX_TX_URBS; i++)
> + priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> +}
> +
> +/* Close USB device */
> +static int usb_8dev_close(struct net_device *netdev)
> +{
> + struct usb_8dev_priv *priv = netdev_priv(netdev);
> + int err = 0;
> +
> + /* Send CLOSE command to CAN controller */
> + err = usb_8dev_cmd_close(priv);
> + if (err)
> + netdev_warn(netdev, "couldn't stop device");
> +
> + priv->can.state = CAN_STATE_STOPPED;
> +
> + netif_stop_queue(netdev);
> +
> + /* Stop polling */
> + unlink_all_urbs(priv);
> +
> + close_candev(netdev);
> +
> + return err;
> +}
> +
> +static const struct net_device_ops usb_8dev_netdev_ops = {
> + .ndo_open = usb_8dev_open,
> + .ndo_stop = usb_8dev_close,
> + .ndo_start_xmit = usb_8dev_start_xmit,
> +};
> +
> +static const struct can_bittiming_const usb_8dev_bittiming_const = {
> + .name = "usb_8dev",
> + .tseg1_min = USB_8DEV_TSEG1_MIN,
> + .tseg1_max = USB_8DEV_TSEG1_MAX,
> + .tseg2_min = USB_8DEV_TSEG2_MIN,
> + .tseg2_max = USB_8DEV_TSEG2_MAX,
> + .sjw_max = USB_8DEV_SJW_MAX,
> + .brp_min = USB_8DEV_BRP_MIN,
> + .brp_max = USB_8DEV_BRP_MAX,
> + .brp_inc = USB_8DEV_BRP_INC,
> +};
> +
> +/*
> + * Probe USB device
> + *
> + * Check device and firmware.
> + * Set supported modes and bittiming constants.
> + * Allocate some memory.
> + */
> +static int usb_8dev_probe(struct usb_interface *intf,
> + const struct usb_device_id *id)
> +{
> + struct net_device *netdev;
> + struct usb_8dev_priv *priv;
> + int i, err = -ENOMEM;
> + u32 version;
> + char buf[18];
> + struct usb_device *usbdev = interface_to_usbdev(intf);
> +
> + /* product id looks strange, better we also check iProduct string */
> + if (usb_string(usbdev, usbdev->descriptor.iProduct, buf,
> +sizeof(buf)) > 0 && strcmp(buf, "USB2CAN converter")) {
> + dev_info(&usbdev->dev, "ignoring: not an USB2CAN converter\n");
> + return -ENODEV;
> + }
> +
> + netdev = alloc_candev(sizeof(struct usb_8dev_priv), MAX_TX_URBS);
> + if (!netdev) {
> + dev_err(&intf->dev, "Couldn't alloc candev\n");
> + return -ENOMEM;
> + }
> +
> + priv = netdev_priv(netdev);
> +
> + priv->udev = usbdev;
> + priv->netdev = netdev;
> +
> + priv->can.state = CAN_STATE_STOPPED;
> + priv->can.clock.freq = USB_8DEV_ABP_CLOCK;
> + priv->can.bittiming_const = &usb_8dev_bittiming_const;
> + priv->can.do_set_mode = usb_8dev_set_mode;
> + priv->can.do_get_berr_counter = usb_8dev_get_berr_counter;
> + priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> + CAN_CTRLMODE_LISTENONLY |
> + CAN_CTRLMODE_ONE_SHOT;
> +
> + netdev->netdev_ops = &usb_8dev_netdev_ops;
> +
> + netdev->flags |= IFF_ECHO; /* we support local echo */
> +
> + init_usb_anchor(&priv->rx_submitted);
> +
> + init_usb_anchor(&priv->tx_submitted);
> + atomic_set(&priv->active_tx_urbs, 0);
> +
> + for (i = 0; i < MAX_TX_URBS; i++)
> + priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> +
> + priv->cmd_msg_buffer = kzalloc(sizeof(struct usb_8dev_cmd_msg),
> + GFP_KERNEL);
> + if (!priv->cmd_msg_buffer) {
> + netdev_err(netdev, "Couldn't alloc Tx buffer\n");
> + goto cleanup_candev;
> + }
> +
> + usb_set_intfdata(intf, priv);
> +
> + SET_NETDEV_DEV(netdev, &intf->dev);
> +
> + mutex_init(&priv->usb_8dev_cmd_lock);
> +
> + err = register_candev(netdev);
> + if (err) {
> + netdev_err(netdev,
> + "couldn't register CAN device: %d\n", err);
> + goto cleanup_cmd_msg_buffer;
> + }
> +
> + err = usb_8dev_cmd_version(priv, &version);
> + if (err) {
> + netdev_err(netdev, "can't get firmware version\n");
> + goto cleanup_cmd_msg_buffer;
> + } else {
> + netdev_info(netdev,
> + "firmware: %d.%d, hardware: %d.%d\n",
> + (version>>24) & 0xff, (version>>16) & 0xff,
> + (version>>8) & 0xff, version & 0xff);
> + }
> +
> + if (device_create_file(&intf->dev, &dev_attr_firmware))
> + netdev_err(netdev,
> + "Couldn't create device file for firmware\n");
> +
> + if (device_create_file(&intf->dev, &dev_attr_hardware))
> + netdev_err(netdev,
> + "Couldn't create device file for hardware\n");
> +
> + return 0;
> +
> +cleanup_cmd_msg_buffer:
> + kfree(priv->cmd_msg_buffer);
> +
> +cleanup_candev:
> + free_candev(netdev);
> +
> + return err;
> +
> +}
> +
> +/* Called by the usb core when driver is unloaded or device is removed */
> +static void usb_8dev_disconnect(struct usb_interface *intf)
> +{
> + struct usb_8dev_priv *priv = usb_get_intfdata(intf);
> +
> + device_remove_file(&intf->dev, &dev_attr_firmware);
> + device_remove_file(&intf->dev, &dev_attr_hardware);
> +
> + usb_set_intfdata(intf, NULL);
> +
> + if (priv) {
> + netdev_info(priv->netdev, "device disconnected\n");
> +
> + unregister_netdev(priv->netdev);
> + free_candev(priv->netdev);
> +
> + unlink_all_urbs(priv);
> + }
> +
> +}
> +
> +static struct usb_driver usb_8dev_driver = {
> + .name = "usb_8dev",
> + .probe =usb_8dev_probe,
> + .disconnect = usb_8dev_disconnect,
> + .id_table = usb_8dev_table,
> +};
> +
> +module_usb_driver(usb_8dev_driver);
> +
> +MODULE_AUTHOR("Bernd Krumboeck ");
> +MODULE_DESCRIPTION("CAN driver for 8 devices USB2CAN interfaces");
> +MODULE_LICENSE("GPL v2");
>
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v7] usb_8dev: Add support for USB2CAN interface from 8 devices
t start device: %d\n",
> + err);
> +
> + close_candev(netdev);
> +
> + return err;
> + }
> +
> + netif_start_queue(netdev);
> +
> + return 0;
> +}
> +
> +static void unlink_all_urbs(struct usb_8dev_priv *priv)
> +{
> + int i;
> +
> + usb_kill_anchored_urbs(&priv->rx_submitted);
> +
> + usb_kill_anchored_urbs(&priv->tx_submitted);
> + atomic_set(&priv->active_tx_urbs, 0);
> +
> + for (i = 0; i < MAX_TX_URBS; i++)
> + priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> +}
> +
> +/* Close USB device */
> +static int usb_8dev_close(struct net_device *netdev)
> +{
> + struct usb_8dev_priv *priv = netdev_priv(netdev);
> + int err = 0;
> +
> + /* Send CLOSE command to CAN controller */
> + err = usb_8dev_cmd_close(priv);
> + if (err)
> + netdev_warn(netdev, "couldn't stop device");
> +
> + priv->can.state = CAN_STATE_STOPPED;
> +
> + netif_stop_queue(netdev);
> +
> + /* Stop polling */
> + unlink_all_urbs(priv);
> +
> + close_candev(netdev);
> +
> + return err;
> +}
> +
> +static const struct net_device_ops usb_8dev_netdev_ops = {
> + .ndo_open = usb_8dev_open,
> + .ndo_stop = usb_8dev_close,
> + .ndo_start_xmit = usb_8dev_start_xmit,
> +};
> +
> +static const struct can_bittiming_const usb_8dev_bittiming_const = {
> + .name = "usb_8dev",
> + .tseg1_min = USB_8DEV_TSEG1_MIN,
> + .tseg1_max = USB_8DEV_TSEG1_MAX,
> + .tseg2_min = USB_8DEV_TSEG2_MIN,
> + .tseg2_max = USB_8DEV_TSEG2_MAX,
> + .sjw_max = USB_8DEV_SJW_MAX,
> + .brp_min = USB_8DEV_BRP_MIN,
> + .brp_max = USB_8DEV_BRP_MAX,
> + .brp_inc = USB_8DEV_BRP_INC,
> +};
> +
> +/*
> + * Probe USB device
> + *
> + * Check device and firmware.
> + * Set supported modes and bittiming constants.
> + * Allocate some memory.
> + */
> +static int usb_8dev_probe(struct usb_interface *intf,
> + const struct usb_device_id *id)
> +{
> + struct net_device *netdev;
> + struct usb_8dev_priv *priv;
> + int i, err = -ENOMEM;
> + u32 version;
> + char buf[18];
> + struct usb_device *usbdev = interface_to_usbdev(intf);
> +
> + /* product id looks strange, better we also check iProduct string */
> + if (usb_string(usbdev, usbdev->descriptor.iProduct, buf,
> +sizeof(buf)) > 0 && strcmp(buf, "USB2CAN converter")) {
> + dev_info(&usbdev->dev, "ignoring: not an USB2CAN converter\n");
> + return -ENODEV;
> + }
> +
> + netdev = alloc_candev(sizeof(struct usb_8dev_priv), MAX_TX_URBS);
> + if (!netdev) {
> + dev_err(&intf->dev, "Couldn't alloc candev\n");
> + return -ENOMEM;
> + }
> +
> + priv = netdev_priv(netdev);
> +
> + priv->udev = usbdev;
> + priv->netdev = netdev;
> +
> + priv->can.state = CAN_STATE_STOPPED;
> + priv->can.clock.freq = USB_8DEV_ABP_CLOCK;
> + priv->can.bittiming_const = &usb_8dev_bittiming_const;
> + priv->can.do_set_mode = usb_8dev_set_mode;
> + priv->can.do_get_berr_counter = usb_8dev_get_berr_counter;
> + priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> + CAN_CTRLMODE_LISTENONLY |
> + CAN_CTRLMODE_ONE_SHOT;
> +
> + netdev->netdev_ops = &usb_8dev_netdev_ops;
> +
> + netdev->flags |= IFF_ECHO; /* we support local echo */
> +
> + init_usb_anchor(&priv->rx_submitted);
> +
> + init_usb_anchor(&priv->tx_submitted);
> + atomic_set(&priv->active_tx_urbs, 0);
> +
> + for (i = 0; i < MAX_TX_URBS; i++)
> + priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> +
> + priv->cmd_msg_buffer = kzalloc(sizeof(struct usb_8dev_cmd_msg),
> + GFP_KERNEL);
> + if (!priv->cmd_msg_buffer) {
> + netdev_err(netdev, "Couldn't alloc Tx buffer\n");
> + goto cleanup_candev;
> + }
> +
> + usb_set_intfdata(intf, priv);
> +
> + SET_NETDEV_DEV(netdev, &intf->dev);
> +
> + mutex_init(&priv->usb_8dev_cmd_lock);
> +
> + err = register_candev(netdev);
> + if (err) {
> + netdev_err(netdev,
> + "couldn't register CAN device: %d\n", err);
> + goto cleanup_cmd_msg_buffer;
> + }
> +
> + err = usb_8dev_cmd_version(priv, &version);
> + if (err) {
> + netdev_err(netdev, "can't get firmware version\n");
> + goto cleanup_cmd_msg_buffer;
> + } else {
> + netdev_info(netdev,
> + "firmware: %d.%d, hardware: %d.%d\n",
> + (version>>24) & 0xff, (version>>16) & 0xff,
> + (version>>8) & 0xff, version & 0xff);
> + }
> +
> + if (device_create_file(&intf->dev, &dev_attr_firmware))
> + netdev_err(netdev,
> + "Couldn't create device file for firmware\n");
> +
> + if (device_create_file(&intf->dev, &dev_attr_hardware))
> + netdev_err(netdev,
> + "Couldn't create device file for hardware\n");
> +
> + return 0;
> +
> +cleanup_cmd_msg_buffer:
> + kfree(priv->cmd_msg_buffer);
> +
> +cleanup_candev:
> + free_candev(netdev);
> +
> + return err;
> +
> +}
> +
> +/* Called by the usb core when driver is unloaded or device is removed */
> +static void usb_8dev_disconnect(struct usb_interface *intf)
> +{
> + struct usb_8dev_priv *priv = usb_get_intfdata(intf);
> +
> + device_remove_file(&intf->dev, &dev_attr_firmware);
> + device_remove_file(&intf->dev, &dev_attr_hardware);
> +
> + usb_set_intfdata(intf, NULL);
> +
> + if (priv) {
> + netdev_info(priv->netdev, "device disconnected\n");
> +
> + unregister_netdev(priv->netdev);
> + free_candev(priv->netdev);
> +
> + unlink_all_urbs(priv);
> + }
> +
> +}
> +
> +static struct usb_driver usb_8dev_driver = {
> + .name = "usb_8dev",
> + .probe =usb_8dev_probe,
> + .disconnect = usb_8dev_disconnect,
> + .id_table = usb_8dev_table,
> +};
> +
> +module_usb_driver(usb_8dev_driver);
> +
> +MODULE_AUTHOR("Bernd Krumboeck ");
> +MODULE_DESCRIPTION("CAN driver for 8 devices USB2CAN interfaces");
> +MODULE_LICENSE("GPL v2");
>
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v7] usb_8dev: Add support for USB2CAN interface from 8 devices
On 12/14/2012 07:22 PM, "Bernd Krumböck" wrote: > Hello Marc! > >> Looks good to me. Minor style issue inline. Please provide a >> documentation for the sysfs entries and place it under: >> Documentation/ABI/testing/sysfs-platform. > > Should I change the sysfs path to /sys/devices/platform/usb_8dev? At the > moment the path is /sys/bus/usb/devices/:. Sorry, my fault. I copied the sysfs path from the at91 driver, which is a platform, not a USB driver. There are several usb descriptions under Documentation/ABI/testing/ see which way to describe your device fits best. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH] usb_8dev: add LED trigger support
On 12/17/2012 10:19 PM, Bernd Krumboeck wrote: > Add support for canbus activity led indicators on usb_8dev devices by > calling appropriate can_led functions. > > These are only enabled when CONFIG_CAN_LEDS is Y, becomes no-op > otherwise. > > Signed-off-by: Bernd Krumboeck Thx. Fabio, can you queue this patch your tree and include it in your series when you send the next iteration? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH] usb_8dev: add LED trigger support
On 12/18/2012 03:18 PM, Fabio Baltieri wrote: > On Tue, Dec 18, 2012 at 03:05:12PM +0100, Marc Kleine-Budde wrote: >> On 12/17/2012 10:19 PM, Bernd Krumboeck wrote: >>> Add support for canbus activity led indicators on usb_8dev devices by >>> calling appropriate can_led functions. >>> >>> These are only enabled when CONFIG_CAN_LEDS is Y, becomes no-op >>> otherwise. >>> >>> Signed-off-by: Bernd Krumboeck >> >> Thx. Fabio, can you queue this patch your tree and include it in your >> series when you send the next iteration? > > Ok I will. Should I add my Signed-off-by to all the patches I'm sending > as I'm on the submissiom path? Yes, correct. Thanks, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On 01/14/2013 11:16 AM, Felipe Balbi wrote:
> Hi,
>
> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
>> @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
>>
>> return fsl_udc_resume(NULL);
>> }
>> -
>> /*-
>> Register entry point for the peripheral controller driver
>> --*/
>> -
>> +static const struct platform_device_id fsl_udc_devtype[] = {
>> +{
>> +.name = "imx-udc-mx25",
>> +.driver_data = IMX25_UDC,
>> +}, {
>> +.name = "imx-udc-mx27",
>> +.driver_data = IMX27_UDC,
>> +}, {
>> +.name = "imx-udc-mx31",
>> +.driver_data = IMX31_UDC,
>> +}, {
>> +.name = "imx-udc-mx35",
>> +.driver_data = IMX35_UDC,
>> +}, {
>> +.name = "imx-udc-mx51",
>> +.driver_data = IMX51_UDC,
>> +}
>> +};
>
> I wonder if your driver-data is actually needed since you can use string
> comparisson to achieve the exact same outcome.
Why use a string compare, if the kernel infrastructure already does this
for you?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On 01/14/2013 11:24 AM, Felipe Balbi wrote:
> On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
>> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
>>>> @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
>>>>
>>>>return fsl_udc_resume(NULL);
>>>> }
>>>> -
>>>>
>>>> /*-
>>>>Register entry point for the peripheral controller driver
>>>>
>>>> --*/
>>>> -
>>>> +static const struct platform_device_id fsl_udc_devtype[] = {
>>>> + {
>>>> + .name = "imx-udc-mx25",
>>>> + .driver_data = IMX25_UDC,
>>>> + }, {
>>>> + .name = "imx-udc-mx27",
>>>> + .driver_data = IMX27_UDC,
>>>> + }, {
>>>> + .name = "imx-udc-mx31",
>>>> + .driver_data = IMX31_UDC,
>>>> + }, {
>>>> + .name = "imx-udc-mx35",
>>>> + .driver_data = IMX35_UDC,
>>>> + }, {
>>>> + .name = "imx-udc-mx51",
>>>> + .driver_data = IMX51_UDC,
>>>> + }
>>>> +};
>>>
>>> I wonder if your driver-data is actually needed since you can use string
>>> comparisson to achieve the exact same outcome.
>>
>> Why use a string compare, if the kernel infrastructure already does this
>> for you?
>
> what do you mean ? What kernel infrastructure is doing waht for me ?
The kernel infrastructure is doing the string compare for you to match
the device against the driver (via platform_device_id->name). You get
the a pointer to the driver_data for free. So you don't need any string
compare in the driver later.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On 01/14/2013 11:39 AM, Felipe Balbi wrote:
> On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
>> On 01/14/2013 11:24 AM, Felipe Balbi wrote:
>>> On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
>>>> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
>>>>> Hi,
>>>>>
>>>>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
>>>>>> @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
>>>>>>
>>>>>> return fsl_udc_resume(NULL);
>>>>>> }
>>>>>> -
>>>>>>
>>>>>> /*-
>>>>>> Register entry point for the peripheral controller driver
>>>>>>
>>>>>> --*/
>>>>>> -
>>>>>> +static const struct platform_device_id fsl_udc_devtype[] = {
>>>>>> +{
>>>>>> +.name = "imx-udc-mx25",
>>>>>> +.driver_data = IMX25_UDC,
>>>>>> +}, {
>>>>>> +.name = "imx-udc-mx27",
>>>>>> +.driver_data = IMX27_UDC,
>>>>>> +}, {
>>>>>> +.name = "imx-udc-mx31",
>>>>>> +.driver_data = IMX31_UDC,
>>>>>> +}, {
>>>>>> +.name = "imx-udc-mx35",
>>>>>> +.driver_data = IMX35_UDC,
>>>>>> +}, {
>>>>>> +.name = "imx-udc-mx51",
>>>>>> +.driver_data = IMX51_UDC,
>>>>>> +}
>>>>>> +};
>>>>>
>>>>> I wonder if your driver-data is actually needed since you can use string
>>>>> comparisson to achieve the exact same outcome.
>>>>
>>>> Why use a string compare, if the kernel infrastructure already does this
>>>> for you?
>>>
>>> what do you mean ? What kernel infrastructure is doing waht for me ?
>>
>> The kernel infrastructure is doing the string compare for you to match
>> the device against the driver (via platform_device_id->name). You get
>> the a pointer to the driver_data for free. So you don't need any string
>> compare in the driver later.
>
> but current driver data is just duplicating name with an integer, it's
> pretty useless driver data.
I don't think so - another argument:
Less code. As struct platform_device_id is a static array the space is
allocated anyway. So it doesn't make any difference if driver_data is
NULL or not. Later you just need to make an integer comparison instead
of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
enum, the compiler will warn you if you've missed an IMX variant.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On 01/14/2013 11:53 AM, Felipe Balbi wrote:
> Hi,
>
> On Mon, Jan 14, 2013 at 11:50:41AM +0100, Marc Kleine-Budde wrote:
>> On 01/14/2013 11:39 AM, Felipe Balbi wrote:
>>> On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
>>>> On 01/14/2013 11:24 AM, Felipe Balbi wrote:
>>>>> On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
>>>>>> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
>>>>>>>> @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device
>>>>>>>> *dev)
>>>>>>>>
>>>>>>>>return fsl_udc_resume(NULL);
>>>>>>>> }
>>>>>>>> -
>>>>>>>>
>>>>>>>> /*-
>>>>>>>>Register entry point for the peripheral controller driver
>>>>>>>>
>>>>>>>> --*/
>>>>>>>> -
>>>>>>>> +static const struct platform_device_id fsl_udc_devtype[] = {
>>>>>>>> + {
>>>>>>>> + .name = "imx-udc-mx25",
>>>>>>>> + .driver_data = IMX25_UDC,
>>>>>>>> + }, {
>>>>>>>> + .name = "imx-udc-mx27",
>>>>>>>> + .driver_data = IMX27_UDC,
>>>>>>>> + }, {
>>>>>>>> + .name = "imx-udc-mx31",
>>>>>>>> + .driver_data = IMX31_UDC,
>>>>>>>> + }, {
>>>>>>>> + .name = "imx-udc-mx35",
>>>>>>>> + .driver_data = IMX35_UDC,
>>>>>>>> + }, {
>>>>>>>> + .name = "imx-udc-mx51",
>>>>>>>> + .driver_data = IMX51_UDC,
>>>>>>>> + }
>>>>>>>> +};
>>>>>>>
>>>>>>> I wonder if your driver-data is actually needed since you can use string
>>>>>>> comparisson to achieve the exact same outcome.
>>>>>>
>>>>>> Why use a string compare, if the kernel infrastructure already does this
>>>>>> for you?
>>>>>
>>>>> what do you mean ? What kernel infrastructure is doing waht for me ?
>>>>
>>>> The kernel infrastructure is doing the string compare for you to match
>>>> the device against the driver (via platform_device_id->name). You get
>>>> the a pointer to the driver_data for free. So you don't need any string
>>>> compare in the driver later.
>>>
>>> but current driver data is just duplicating name with an integer, it's
>>> pretty useless driver data.
>>
>> I don't think so - another argument:
>> Less code. As struct platform_device_id is a static array the space is
>> allocated anyway. So it doesn't make any difference if driver_data is
>> NULL or not. Later you just need to make an integer comparison instead
>> of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
>> enum, the compiler will warn you if you've missed an IMX variant.
>
> fair enough, but then don't create a different enum value for each imx
> instance if they're mostly the same. Differentiate only what's actually
> different.
Usually there isn't any Changelog between IP cores used in the different
fsl processors (at least available outside of fsl), that makes it quite
difficult to say if something found on one imx is really the same as on
the other one. And they (usually) don't provide any versioning
information in a register or the documentation.
just my 2¢
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On 01/14/2013 06:40 PM, Felipe Balbi wrote: > Hi, > > On Mon, Jan 14, 2013 at 08:56:33PM +0800, Peter Chen wrote: > > > >>>> Usually there isn't any Changelog between IP cores used in the different >>>> fsl processors (at least available outside of fsl), that makes it quite >>>> difficult to say if something found on one imx is really the same as on >>>> the other one. And they (usually) don't provide any versioning >>>> information in a register or the documentation. >>>> >>>> just my 2¢ >>> >>> $SUBJECT is trying to differentiate a single feature (or maybe two) to >>> replace cpu_is_xxx(), then expose that on driver_data without creating >>> one enum value for each release from fsl. >> >> Felipe, every one or two SoCs may have their special operations for >> integrate PHY interface, clk operation, or workaround for IC >> limitation. > > the particular PHY and clk used should be hidden by phy layer and clk > API respectively. Workarounds, fair enough, we need to handle them; but > ideally those should be based on runtime revision detection, not some > hackery using driver_data. If this is actually possible, I'd love to do this. But IP vendor don't include a version register in their cores. :( >> Maybe, it will add more future or SoCs (maybe not for this driver) in >> the future, using enum is easier than string comparison for expanding >> something. > > a) I never told you to *not* use enum. I said that creating DEVICE_A, > DEVICE_B, DEVICE_C, DEVICE_D and DEVICE_E values when DEVICE_B, > DEVICE_C and DEVICE_E behave exactly the same is unnecessary. > > b) you can't be expecting to add future SoCs support to fsl udc, I have > already said and will repeat for the last time: move to chipidea ASAP. > > New SoCs cannot be added to fsl udc, you *must* use chipidea for > anything new and move the legacy to chipidea eventually. I will wait for > a full year for you to do that, but after that I will have to start > deleting drivers for the sake of avoid duplication of effort. +1 Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v9] usb_8dev: Add support for USB2CAN interface from 8 devices
On 01/14/2013 09:59 PM, Oliver Hartkopp wrote: > Hi Bernd, > > On 16.12.2012 08:01, Bernd Krumboeck wrote: > >> Add device driver for USB2CAN interface from "8 devices" >> (http://www.8devices.com). >> >> changes since v8: >> * remove all sysfs files >> >> changes since v7: >> * add sysfs documentation >> * fix minor styling issue >> * fixed can state for passive mode >> * changed handling for crc errors >> >> changes since v6: >> * changed some variable types to big endian equivalent >> * small cleanups >> >> changes since v5: >> * unlock mutex on error >> >> changes since v4: >> * removed FSF address >> * renamed struct usb_8dev >> * removed unused variable free_slots >> * replaced some _to_cpu functions with pointer equivalent >> * fix return value for usb_8dev_set_mode >> * handle can errors with separate function >> * fix overrun error handling >> * rewrite error handling for usb_8dev_start_xmit >> * fix urb submit in usb_8dev_start >> * various small fixes >> >> Signed-off-by: Bernd Krumboeck >> Acked-by: Wolfgang Grandegger > > > Tested-by: Oliver Hartkopp > > Fortunately i got my adapter today ... > > [ 52.984254] usb 2-1.4: new full-speed USB device number 6 using ehci-pci > [ 53.078690] usb 2-1.4: New USB device found, idVendor=0483, idProduct=1234 > [ 53.078698] usb 2-1.4: New USB device strings: Mfr=1, Product=2, > SerialNumber=3 > [ 53.078703] usb 2-1.4: Product: USB2CAN converter > [ 53.078707] usb 2-1.4: Manufacturer: edevices > [ 53.078711] usb 2-1.4: SerialNumber: ED000212 > [ 53.111990] usb_8dev 2-1.4:1.0 can2: firmware: 1.4, hardware: 255.255 > [ 53.112090] usbcore: registered new interface driver usb_8dev > > Looks good :-) > > When detaching the device from the CAN bus when sending/receiving CAN traffic > i got these dmesg infos: > > [ 960.047130] usb_8dev 2-1.4:1.0 can2: Unknown status/error message (0) > [ 976.544343] usb_8dev 2-1.4:1.0 can2: Unknown status/error message (0) > > Did you check these kind of 'unfriendly user' tests? > > E.g. pulling the USB under receive load brings this output: > > [ 1343.786427] usb_8dev 2-1.4:1.0 can2: Rx URB aborted (-32) > [ 1343.786640] usb_8dev 2-1.4:1.0 can2: Rx URB aborted (-32) > > (..) another 344 of these URB aborted messages > > [ 1343.872620] usb_8dev 2-1.4:1.0 can2: Rx URB aborted (-32) > [ 1343.872867] usb_8dev 2-1.4:1.0 can2: Rx URB aborted (-32) > [ 1343.873124] usb_8dev 2-1.4:1.0 can2: Rx URB aborted (-32) > [ 1343.873269] usb 2-1.4: USB disconnect, device number 6 > [ 1343.873363] usb_8dev 2-1.4:1.0 can2: Rx URB aborted (-32) > [ 1343.875259] usb_8dev 2-1.4:1.0 can2: device disconnected > [ 1343.875366] usb_8dev 2-1.4:1.0 can2: sending command message failed > [ 1343.875371] usb_8dev 2-1.4:1.0 can2: couldn't stop device > > > Tomorrow i will do some more testing. > > The LED handling of the device is really fine: > > - interface down -> red > - interface up -> green > - interface error passive -> green blinking > > Regards, > Oliver What do you suggest? Add the driver or fix the errors first? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v9] usb_8dev: Add support for USB2CAN interface from 8 devices
On 01/15/2013 09:30 PM, Oliver Hartkopp wrote: > On 15.01.2013 21:23, Marc Kleine-Budde wrote: > >> On 01/14/2013 09:59 PM, Oliver Hartkopp wrote: > > >> >> What do you suggest? Add the driver or fix the errors first? > > > Hm. > > We're currently at 3.8-rc3 - so waiting for reactions from Bernd makes more > sense to me. okay > That's better than sending a pull request that includes fixes for brand new > drivers %-) :) Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v11] usb_8dev: Add support for USB2CAN interface from 8 devices
On 01/19/2013 07:30 AM, Bernd Krumboeck wrote: > Add device driver for USB2CAN interface from "8 devices" > (http://www.8devices.com). > > changes since v10: > * small cleanups > > changes since v9: > * fixed syslog messages > * fixed crc error number > * increased MAX_RX_URBS and MAX_TX_URBS > > changes since v8: > * remove all sysfs files > > changes since v7: > * add sysfs documentation > * fix minor styling issue > * fixed can state for passive mode > * changed handling for crc errors > > changes since v6: > * changed some variable types to big endian equivalent > * small cleanups > > changes since v5: > * unlock mutex on error > > changes since v4: > * removed FSF address > * renamed struct usb_8dev > * removed unused variable free_slots > * replaced some _to_cpu functions with pointer equivalent > * fix return value for usb_8dev_set_mode > * handle can errors with separate function > * fix overrun error handling > * rewrite error handling for usb_8dev_start_xmit > * fix urb submit in usb_8dev_start > * various small fixes > > Signed-off-by: Bernd Krumboeck > Acked-by: Wolfgang Grandegger > Tested-by: Oliver Hartkopp Thanks for you patience :) Applied to can-next. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v11] usb_8dev: Add support for USB2CAN interface from 8 devices
On 01/22/2013 10:34 PM, Marc Kleine-Budde wrote: > On 01/19/2013 07:30 AM, Bernd Krumboeck wrote: >> Add device driver for USB2CAN interface from "8 devices" >> (http://www.8devices.com). >> >> changes since v10: >> * small cleanups >> >> changes since v9: >> * fixed syslog messages >> * fixed crc error number >> * increased MAX_RX_URBS and MAX_TX_URBS >> >> changes since v8: >> * remove all sysfs files >> >> changes since v7: >> * add sysfs documentation >> * fix minor styling issue >> * fixed can state for passive mode >> * changed handling for crc errors >> >> changes since v6: >> * changed some variable types to big endian equivalent >> * small cleanups >> >> changes since v5: >> * unlock mutex on error >> >> changes since v4: >> * removed FSF address >> * renamed struct usb_8dev >> * removed unused variable free_slots >> * replaced some _to_cpu functions with pointer equivalent >> * fix return value for usb_8dev_set_mode >> * handle can errors with separate function >> * fix overrun error handling >> * rewrite error handling for usb_8dev_start_xmit >> * fix urb submit in usb_8dev_start >> * various small fixes >> >> Signed-off-by: Bernd Krumboeck >> Acked-by: Wolfgang Grandegger >> Tested-by: Oliver Hartkopp > > Thanks for you patience :) Applied to can-next. I've tweaked some comments to make checkpatch a bit more quiet. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH,RFC] usb: add devicetree helpers for determining dr_mode and phy_type
On 01/29/2013 03:55 PM, Wolfram Sang wrote: > >> I prefer u32 here, because we have the matching enum. Otherwise we end >> up with: >> >> of_property_read_string(...,&type); >> >> if (!strcmp(type, "ulpi")) >> foo(); >> else if (!strcmp(type, "utmi")) >> bar(); >> else if (!strcmp(type, "pipe3")) >> baz(); >> else >> BUG(); >> >> and I don't like that, it's ugly and error prone. > > Error prone? I guess my mileage varies. Especially compared to the > probability devicetree creators pick the wrong number. > > It also removes the (probably implicit) rule that the enum mustn't be > modified since it is exported to users. > > Also, you could map the strings to the enum first and then switch-case > over it to make the code nicer. That's what the code already does. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH, RFC] usb: add devicetree helpers for determining dr_mode and phy_type
On 01/29/2013 06:11 PM, Stephen Warren wrote: > On 01/29/2013 04:22 AM, Sascha Hauer wrote: >> From: Michael Grzeschik >> >> This adds two little devicetree helper functions for determining the >> dr_mode (host, peripheral, otg) and phy_type (utmi, ulpi,...) from >> the devicetree. >> >> Signed-off-by: Michael Grzeschik >> Signed-off-by: Marc Kleine-Budde >> --- >> >> The properties and their values have been taken from the fsl-mph-dr driver. >> This binding is also documented (though currently not used) for the tegra >> ehci driver (Documentation/devicetree/bindings/usb/nvidia,tegra20-ehci.txt). >> This is a first attempt to parse these bindings at a common place so that >> others can make use of it. >> >> Basically I want to know whether this binding is recommended for new drivers >> since normally the devicetree uses '-' instead of '_', and maybe there are >> other problems with it. > > It's certainly typical to use - not _ for freshly defined properties. > However, since this property already exists and is in-use, I don't think > there's any choice but to maintain its current definition. > > The code looked fine to me. The code for phy_type is lifted from the ethernet guys and adopted to the usb phy types. "dr_mode" is, as Sascha pointed out, only used in drivers/usb/host/fsl-mph-dr-of.c Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v2 1/6] usb: otg: Add an API to bind the USB controller and PHY
On 01/25/2013 03:33 AM, Kishon Vijay Abraham I wrote:
> In order to support platforms which has multiple PHY's (of same type) and
> which has multiple USB controllers, a new design is adopted wherin the binding
> information (between the PHY and the USB controller) should be passed to the
> PHY library from platform specific file (board file).
> So added a new API to pass the binding information.
>
> Signed-off-by: Kishon Vijay Abraham I
> ---
> drivers/usb/otg/otg.c | 37 +
> include/linux/usb/phy.h | 22 ++
> 2 files changed, 59 insertions(+)
>
> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
> index a30c041..8e756d9 100644
> --- a/drivers/usb/otg/otg.c
> +++ b/drivers/usb/otg/otg.c
> @@ -18,6 +18,7 @@
> #include
>
> static LIST_HEAD(phy_list);
> +static LIST_HEAD(phy_bind_list);
> static DEFINE_SPINLOCK(phy_lock);
>
> static struct usb_phy *__usb_find_phy(struct list_head *list,
> @@ -201,6 +202,42 @@ void usb_remove_phy(struct usb_phy *x)
> }
> EXPORT_SYMBOL(usb_remove_phy);
>
> +/**
> + * usb_bind_phy - bind the phy and the controller that uses the phy
> + * @dev_name: the device name of the device that will bind to the phy
> + * @index: index to specify the port number
> + * @phy_dev_name: the device name of the phy
> + *
> + * Fills the phy_bind structure with the dev_name and phy_dev_name. This will
> + * be used when the phy driver registers the phy and when the controller
> + * requests this phy.
> + *
> + * To be used by platform specific initialization code.
> + */
> +int __init usb_bind_phy(const char *dev_name, u8 index,
> + const char *phy_dev_name)
> +{
> + struct usb_phy_bind *phy_bind;
> + unsigned long flags;
> +
> + phy_bind = kzalloc(sizeof(*phy_bind), GFP_KERNEL);
> + if (!phy_bind) {
> + pr_err("phy_bind(): No memory for phy_bind");
> + return -ENOMEM;
> + }
> +
> + phy_bind->dev_name = dev_name;
> + phy_bind->phy_dev_name = phy_dev_name;
> + phy_bind->index = index;
> +
> + spin_lock_irqsave(&phy_lock, flags);
> + list_add_tail(&phy_bind->list, &phy_bind_list);
> + spin_unlock_irqrestore(&phy_lock, flags);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_bind_phy);
> +
> const char *otg_state_string(enum usb_otg_state state)
> {
> switch (state) {
> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
> index a29ae1e..e7eb429 100644
> --- a/include/linux/usb/phy.h
> +++ b/include/linux/usb/phy.h
> @@ -106,6 +106,21 @@ struct usb_phy {
> enum usb_device_speed speed);
> };
>
> +/**
> + * struct usb_phy_bind - represent the binding for the phy
> + * @dev_name: the device name of the device that will bind to the phy
> + * @phy_dev_name: the device name of the phy
> + * @index: used if a single controller uses multiple phys
> + * @phy: reference to the phy
> + * @list: to maintain a linked list of the binding information
> + */
> +struct usb_phy_bind {
> + const char *dev_name;
> + const char *phy_dev_name;
> + u8 index;
> + struct usb_phy *phy;
> + struct list_head list;
> +};
>
> /* for board-specific init logic */
> extern int usb_add_phy(struct usb_phy *, enum usb_phy_type type);
> @@ -151,6 +166,8 @@ extern struct usb_phy *devm_usb_get_phy(struct device
> *dev,
> enum usb_phy_type type);
> extern void usb_put_phy(struct usb_phy *);
> extern void devm_usb_put_phy(struct device *dev, struct usb_phy *x);
> +extern int usb_bind_phy(const char *dev_name, u8 index,
> + const char *phy_dev_name);
> #else
> static inline struct usb_phy *usb_get_phy(enum usb_phy_type type)
> {
> @@ -171,6 +188,11 @@ static inline void devm_usb_put_phy(struct device *dev,
> struct usb_phy *x)
> {
> }
>
> +static inline int usb_bind_phy(const char *dev_name, u8 index,
> + const char *phy_dev_name)
> +{
> + return NULL;
The return value looks bogus.
Marc
> +}
> #endif
>
> static inline int
>
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
[PATCH] usb: otg: use try_module_get in all usb_get_phy functions and add missing module_put
In patch "5d3c28b usb: otg: add device tree support to otg library"
devm_usb_get_phy_by_phandle() was added. It uses try_module_get() to lock the
phy driver in memory. The corresponding module_put() is missing in that patch.
This patch adds try_module_get() to usb_get_phy() and usb_get_phy_dev().
Further the missing module_put() is added to usb_put_phy().
Signed-off-by: Marc Kleine-Budde
---
Hello,
this patch applies to Greg's usb-next tree. I hope the module counter is now
consistent.
Marc
drivers/usb/otg/otg.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
index e181439..2bd03d2 100644
--- a/drivers/usb/otg/otg.c
+++ b/drivers/usb/otg/otg.c
@@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
spin_lock_irqsave(&phy_lock, flags);
phy = __usb_find_phy(&phy_list, type);
- if (IS_ERR(phy)) {
+ if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
pr_err("unable to find transceiver of type %s\n",
usb_phy_type_string(type));
goto err0;
@@ -228,7 +228,7 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8
index)
spin_lock_irqsave(&phy_lock, flags);
phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
- if (IS_ERR(phy)) {
+ if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
pr_err("unable to find transceiver\n");
goto err0;
}
@@ -301,8 +301,12 @@ EXPORT_SYMBOL(devm_usb_put_phy);
*/
void usb_put_phy(struct usb_phy *x)
{
- if (x)
+ if (x) {
+ struct module *owner = x->dev->driver->owner;
+
put_device(x->dev);
+ module_put(owner);
+ }
}
EXPORT_SYMBOL(usb_put_phy);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: otg: use try_module_get in all usb_get_phy functions and add missing module_put
On 02/01/2013 10:48 PM, Marc Kleine-Budde wrote: > In patch "5d3c28b usb: otg: add device tree support to otg library" > devm_usb_get_phy_by_phandle() was added. It uses try_module_get() to lock the > phy driver in memory. The corresponding module_put() is missing in that patch. > > This patch adds try_module_get() to usb_get_phy() and usb_get_phy_dev(). > Further the missing module_put() is added to usb_put_phy(). > > Signed-off-by: Marc Kleine-Budde If this patch is okay, I suggest that Sascha takes it in his patch series before moving the function from otg.c Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v2 2/6] usb: otg: utils: add facilities in phy lib to support multiple PHYs of same type
On 01/25/2013 03:33 AM, Kishon Vijay Abraham I wrote:
> In order to add support for multipe PHY's of the same type, new API's
> for adding PHY and getting PHY has been added. Now the binding
> information for the PHY and controller should be done in platform file
> using usb_bind_phy API. And for getting a PHY, the device pointer of the
> USB controller and an index should be passed. Based on the binding
> information that is added in the platform file, usb_get_phy_dev will return
> the
> appropriate PHY.
> Already existing API's to add and get phy by type is not removed. These
> API's are deprecated and will be removed once all the platforms start to
> use the new API.
>
> Signed-off-by: Kishon Vijay Abraham I
> ---
> drivers/usb/otg/otg.c | 118
> ++-
> include/linux/usb/phy.h | 13 ++
> 2 files changed, 130 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
> index 8e756d9..4bb4333 100644
> --- a/drivers/usb/otg/otg.c
> +++ b/drivers/usb/otg/otg.c
> @@ -36,6 +36,24 @@ static struct usb_phy *__usb_find_phy(struct list_head
> *list,
> return ERR_PTR(-ENODEV);
> }
>
> +static struct usb_phy *__usb_find_phy_dev(struct device *dev,
> + struct list_head *list, u8 index)
> +{
> + struct usb_phy_bind *phy_bind = NULL;
> +
> + list_for_each_entry(phy_bind, list, list) {
> + if (!(strcmp(phy_bind->dev_name, dev_name(dev))) &&
> + phy_bind->index == index) {
> + if (phy_bind->phy)
> + return phy_bind->phy;
> + else
> + return ERR_PTR(-EPROBE_DEFER);
> + }
> + }
> +
> + return ERR_PTR(-ENODEV);
> +}
> +
> static void devm_usb_phy_release(struct device *dev, void *res)
> {
> struct usb_phy *phy = *(struct usb_phy **)res;
> @@ -112,6 +130,69 @@ err0:
> EXPORT_SYMBOL(usb_get_phy);
>
> /**
> + * usb_get_phy_dev - find the USB PHY
> + * @dev - device that requests this phy
> + * @index - the index of the phy
> + *
> + * Returns the phy driver, after getting a refcount to it; or
> + * -ENODEV if there is no such phy. The caller is responsible for
> + * calling usb_put_phy() to release that count.
> + *
> + * For use by USB host and peripheral drivers.
> + */
> +struct usb_phy *usb_get_phy_dev(struct device *dev, u8 index)
> +{
> + struct usb_phy *phy = NULL;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&phy_lock, flags);
> +
> + phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
> + if (IS_ERR(phy)) {
You should probably lock the phy module in memory. See my patch "usb:
otg: use try_module_get in all usb_get_phy functions and add missing
module_put".
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH 1/9] usb: otg: use try_module_get in all usb_get_phy functions and add missing module_put
On 02/04/2013 02:59 PM, Roger Quadros wrote:
> On 02/04/2013 03:24 PM, Sascha Hauer wrote:
>> From: Marc Kleine-Budde
>>
>> In patch "5d3c28b usb: otg: add device tree support to otg library"
>> devm_usb_get_phy_by_phandle() was added. It uses try_module_get() to lock the
>> phy driver in memory. The corresponding module_put() is missing in that
>> patch.
>>
>> This patch adds try_module_get() to usb_get_phy() and usb_get_phy_dev().
>> Further the missing module_put() is added to usb_put_phy().
>>
>> Signed-off-by: Marc Kleine-Budde
>> Signed-off-by: Sascha Hauer
>> ---
>> drivers/usb/otg/otg.c | 10 +++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
>> index e181439..2bd03d2 100644
>> --- a/drivers/usb/otg/otg.c
>> +++ b/drivers/usb/otg/otg.c
>> @@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
>> spin_lock_irqsave(&phy_lock, flags);
>>
>> phy = __usb_find_phy(&phy_list, type);
>> -if (IS_ERR(phy)) {
>> +if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
>> pr_err("unable to find transceiver of type %s\n",
>> usb_phy_type_string(type));
>> goto err0;
>
> There are 2 problems with this.
>
> - If phy was found but try_module_get failed you are not returning error code.
Correct - but
> - If phy was found and try_module_get fails we would want to use deferred
> probing,
> since this case is possible if the phy module is not yet loaded and alive but
> can be in
> the future.
...this should not happen, as the phy list is protected by the
spin_lock. If a phy driver module has added a the phy to the
infrastructure, but has been rmmod'ed without removing the phy. The phy
driver is broken anyway.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH] drivers: net: Remove remaining alloc/OOM messages
On 02/07/2013 10:46 PM, Joe Perches wrote: > alloc failures already get standardized OOM > messages and a dump_stack. > > For the affected mallocs around these OOM messages: > > Converted kmallocs with multiplies to kmalloc_array. > Converted a kmalloc/memcpy to kmemdup. > Removed now unused stack variables. > Removed unnecessary parentheses. > Neatened alignment. > > Signed-off-by: Joe Perches > --- > Let me know if you want multiple small patches instead. > > drivers/net/can/usb/ems_usb.c | 4 +- For ems_usb.c Acked-by: Marc Kleine-Budde regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type
On 02/14/2013 10:36 AM, Alexander Shishkin wrote:
> Sascha Hauer writes:
>
>> From: Michael Grzeschik
>>
>> This adds two little devicetree helper functions for determining the
>> dr_mode (host, peripheral, otg) and phy_type (utmi, ulpi,...) from
>> the devicetree.
>>
>> Signed-off-by: Michael Grzeschik
>> Signed-off-by: Marc Kleine-Budde
>> Signed-off-by: Sascha Hauer
>> ---
>> drivers/usb/phy/Makefile |1 +
>> drivers/usb/phy/of.c | 47
>> ++
>> drivers/usb/usb-common.c | 36 +++
>> include/linux/usb/of.h | 27 ++
>> include/linux/usb/otg.h |7 +++
>> include/linux/usb/phy.h |9 +
>> 6 files changed, 127 insertions(+)
>> create mode 100644 drivers/usb/phy/of.c
>> create mode 100644 include/linux/usb/of.h
>>
>> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
>> index 9fa6327..e1be1e8 100644
>> --- a/drivers/usb/phy/Makefile
>> +++ b/drivers/usb/phy/Makefile
>> @@ -5,6 +5,7 @@
>> ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
>>
>> obj-$(CONFIG_USB_OTG_UTILS) += phy.o
>> +obj-$(CONFIG_OF)+= of.o
>> obj-$(CONFIG_OMAP_USB2) += omap-usb2.o
>> obj-$(CONFIG_OMAP_USB3) += omap-usb3.o
>> obj-$(CONFIG_OMAP_CONTROL_USB) += omap-control-usb.o
>> diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c
>> new file mode 100644
>> index 000..e6f3b74
>> --- /dev/null
>> +++ b/drivers/usb/phy/of.c
>> @@ -0,0 +1,47 @@
>> +/*
>> + * USB of helper code
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +static const char *usbphy_modes[] = {
>> +[USBPHY_INTERFACE_MODE_UNKNOWN] = "",
>> +[USBPHY_INTERFACE_MODE_UTMI]= "utmi",
>> +[USBPHY_INTERFACE_MODE_UTMIW] = "utmi_wide",
>> +[USBPHY_INTERFACE_MODE_ULPI]= "ulpi",
>> +[USBPHY_INTERFACE_MODE_SERIAL] = "serial",
>> +[USBPHY_INTERFACE_MODE_HSIC]= "hsic",
>> +};
>> +
>> +/**
>> + * of_usb_get_phy_mode - Get phy mode for given device_node
>> + * @np: Pointer to the given device_node
>> + *
>> + * The function gets phy interface string from property 'phy_type',
>> + * and returns the correspondig enum usb_phy_interface
>> + */
>> +enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np)
>> +{
>> +const char *phy_type;
>> +int err, i;
>> +
>> +err = of_property_read_string(np, "phy_type", &phy_type);
>> +if (err < 0)
>> +return USBPHY_INTERFACE_MODE_UNKNOWN;
>> +
>> +for (i = 0; i < ARRAY_SIZE(usbphy_modes); i++)
>> +if (!strcmp(phy_type, usbphy_modes[i]))
>> +return i;
>> +
>> +return USBPHY_INTERFACE_MODE_UNKNOWN;
>> +}
>> +EXPORT_SYMBOL_GPL(of_usb_get_phy_mode);
>> diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
>> index d29503e..ad4d87d 100644
>> --- a/drivers/usb/usb-common.c
>> +++ b/drivers/usb/usb-common.c
>> @@ -14,6 +14,9 @@
>> #include
>> #include
>> #include
>> +#include
>> +#include
>> +#include
>>
>> const char *usb_speed_string(enum usb_device_speed speed)
>> {
>> @@ -32,4 +35,37 @@ const char *usb_speed_string(enum usb_device_speed speed)
>> }
>> EXPORT_SYMBOL_GPL(usb_speed_string);
>>
>> +#ifdef CONFIG_OF
>> +static const char *usb_dr_modes[] = {
>> +[USB_DR_MODE_UNKNOWN] = "",
>> +[USB_DR_MODE_HOST] = "host",
>> +[USB_DR_MODE_PERIPHERAL]= "peripheral",
>> +[USB_DR_MODE_OTG] = "otg",
>> +};
>
> It turns out this is a problem, especially since this is generic usb
> code: we have a chipidea controller (a patchset just arrived) that does
> both host and peripheral, but not otg. And I'm told now that dwc3
> controller can be synthesized like that too.
You mean a single instance of the controller (i.e. USB port) is host and
peripheral but has no otg registers. This means the mode of the port is
configured by userspace via the debugfs file? Is this possible?
The above property describes a single port not the whole controller. If
there is a controller with one host and one peripheral port the code in
this patch should be sufficient, as you have a property in the DT for
each port.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type
On 02/14/2013 10:58 AM, Felipe Balbi wrote:
> Hi,
>
> On Thu, Feb 14, 2013 at 10:49:44AM +0100, Marc Kleine-Budde wrote:
>>>> diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
>>>> index d29503e..ad4d87d 100644
>>>> --- a/drivers/usb/usb-common.c
>>>> +++ b/drivers/usb/usb-common.c
>>>> @@ -14,6 +14,9 @@
>>>> #include
>>>> #include
>>>> #include
>>>> +#include
>>>> +#include
>>>> +#include
>>>>
>>>> const char *usb_speed_string(enum usb_device_speed speed)
>>>> {
>>>> @@ -32,4 +35,37 @@ const char *usb_speed_string(enum usb_device_speed
>>>> speed)
>>>> }
>>>> EXPORT_SYMBOL_GPL(usb_speed_string);
>>>>
>>>> +#ifdef CONFIG_OF
>>>> +static const char *usb_dr_modes[] = {
>>>> + [USB_DR_MODE_UNKNOWN] = "",
>>>> + [USB_DR_MODE_HOST] = "host",
>>>> + [USB_DR_MODE_PERIPHERAL]= "peripheral",
>>>> + [USB_DR_MODE_OTG] = "otg",
>>>> +};
>>>
>>> It turns out this is a problem, especially since this is generic usb
>>> code: we have a chipidea controller (a patchset just arrived) that does
>>> both host and peripheral, but not otg. And I'm told now that dwc3
>>> controller can be synthesized like that too.
>
> I wonder if this part is really necessary. Usually you would read it
> from HW's registers. For dwc3, it's quite recently that we allowed the
> driver to be built with host-only, device-only or DRD functionality.
The imx25 USB version of the chipidea IP Core get's really confused if
you read from the CAP_DCCPARAMS register on the host only port. In
freescale's documentation the register is marked as reserved. The host
port will not work then.
> Also, this is likely to create troubles if not done correctly. Imagine
> user compiles a host-only driver and board passes dr_mode = peripheral ?
Use dev_err() and inform the user.
> Maybe we can ignore dr_mode in host-only and device-only builds and only
> look at it for DRD builds ?
>
>> You mean a single instance of the controller (i.e. USB port) is host and
>> peripheral but has no otg registers. This means the mode of the port is
>> configured by userspace via the debugfs file? Is this possible?
>
> yes, it is possible. Dual-Role doesn't imply OTG, but OTG implies
> Dual-Role.
Thanks for the info. Then we need a fourth value for the helper code.
What's a sensible sting: "dual-role", "dr"?
>> The above property describes a single port not the whole controller. If
>> there is a controller with one host and one peripheral port the code in
>> this patch should be sufficient, as you have a property in the DT for
>> each port.
>
> I don't think you can have a single controller like that, but good that
> it supports.
The DT binding is per USB port, so that's a by product of good
abstraction :)
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v2 0/5] Generic PHY Framework
On 02/19/2013 04:05 PM, Felipe Balbi wrote: > Hi, > > On Tue, Feb 19, 2013 at 02:34:40PM +, Arnd Bergmann wrote: >> On Tuesday 19 February 2013, Felipe Balbi wrote: >>> On Tue, Feb 19, 2013 at 12:33:54PM +, Arnd Bergmann wrote: >>>>> Currently drivers/phy and drivers/net/phy are independent and are not >>>>> related to each other. There are some fundamental differences on how >>>>> these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to >>>>> match a PHY device with a PHY driver and the Ethernet device uses the >>>>> bus layer to get the PHY. >>>>> The Generic PHY Framework however doesn't have any bus layer. The PHY >>>>> should be like any other Platform Devices and Drivers and the framework >>>>> will provide some APIs to register with the framework. And there are >>>>> other APIs which any controller can use to get the PHY (for e.g., in the >>>>> case of dt boot, it can use phandle to get a reference to the PHY). >>>> >>>> Hmm, I think the use of a bus_type for a PHY actually sounds quite >>>> appropriate. The actual PHY device would then be a child of the >>> >>> really ? I'm not so sure, the *bus* used by the PHY is ULPI, UTMI, >>> UTMI+, PIP3, I2C, etc... adding another 'fake' bus representation is a >>> bit overkill IMO. >>> >>> Imagine an I2C-controlled PHY driver like isp1301, that driver will have >>> to register i2c_driver and phy_driver, which looks weird to me. If the >>> only substitute for class is a bus, we can't drop classes just yet, I'm >>> afraid. >>> >>> Imagine a regulator bus, a pwm bus, an LED bus etc. They don't make >>> sense IMHO. >> >> It's a fine line, but I think a phy is something that resembles a device >> more than an LED does. When I read patch 1, I also noticed and commented >> on the fact that it does use a 'class'. Now, according to Greg, we should >> use 'bus_type' instead of 'class' in new code. I originally disagreed with >> that concept, but he's the boss here and it's good if he has a vision >> how things should be lined out. >> >> In practice, there is little difference between a 'bus_type' and a 'class', >> so just replace any instance of the former with the latter in your head >> when reading the code ;-) > > it's not so simple :-) if we must use bus_type we need to introduce all > the device/driver matching mechanism which isn't necessary with a class. You have the code for phy <-> device matching in your framework anyway. Using the bus infrastructure brings changes the open coded matching into bus callbacks. regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH] usb: otg: use try_module_get in all usb_get_phy functions and add missing module_put
On 02/25/2013 05:54 PM, Felipe Balbi wrote: >> Hello, >> >> this patch got lost somehow, it applies to current linus/master (pre >> v3.9-rc1). >> Please apply. > > I'll take it once v3.9-rc1 is tagged, thanks tnx, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: About chipidea tree
On 02/26/2013 11:56 AM, Alexander Shishkin wrote: > Peter Chen writes: > >> Hi Alex, > > Hi, > >> Do we have a chipidea repo which is queued for mainline? >> We have several patchsets for chipidea these monthes, I >> don't know their status. For me, I would like based >> on your tree if it exists. > > I thought about it, but then it seems like having a separate branch is > bound to be confusing to most people. I'd much rather prefer everything > go to usb-next, and this is my current plan. Since Greg will start > applying new stuff to usb-next after -rc1 is tagged, I'll send my > current stash of patches for inclusion then. If your patchset, for Can we have a look at your queued patches? > example, has conflicts with my stuff that's not merged, I'll try to take > care of resolving the conflicts and submit everything to Greg. In other > words, it should be always ok to base your chipidea patchsets on > usb-next. > > Let me know if this sounds reasonable to you. Michael has already done that work (some S-o-b form Michael are missing) and rebased Sascha's and Peter's patches to current linus/master, see this tree : http://git.pengutronix.de/?p=mgr/linux.git;a=shortlog;h=refs/heads/chipidea-for-v3.10 The tree includes my patch we resent yesterday, as Sascha's series depends on this. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: About chipidea tree
On 02/26/2013 02:25 PM, Alexander Shishkin wrote: > Marc Kleine-Budde writes: > >> On 02/26/2013 11:56 AM, Alexander Shishkin wrote: >>> Peter Chen writes: >>> >>>> Hi Alex, >>> >>> Hi, >>> >>>> Do we have a chipidea repo which is queued for mainline? >>>> We have several patchsets for chipidea these monthes, I >>>> don't know their status. For me, I would like based >>>> on your tree if it exists. >>> >>> I thought about it, but then it seems like having a separate branch is >>> bound to be confusing to most people. I'd much rather prefer everything >>> go to usb-next, and this is my current plan. Since Greg will start >>> applying new stuff to usb-next after -rc1 is tagged, I'll send my >>> current stash of patches for inclusion then. If your patchset, for >> >> Can we have a look at your queued patches? > > Sure, > http://marc.info/?l=linux-usb&m=135902434508839 > >>> example, has conflicts with my stuff that's not merged, I'll try to take >>> care of resolving the conflicts and submit everything to Greg. In other >>> words, it should be always ok to base your chipidea patchsets on >>> usb-next. >>> >>> Let me know if this sounds reasonable to you. >> >> Michael has already done that work (some S-o-b form Michael are missing) >> and rebased Sascha's and Peter's patches to current linus/master, see >> this tree : >> >> http://git.pengutronix.de/?p=mgr/linux.git;a=shortlog;h=refs/heads/chipidea-for-v3.10 > > Taking a quick look, quite some of those patches are not ready for > inclusion yet. So if the question is, do we need a -next tree for all > the chipidea patchsets floating around, it might be a good thing. But > it's not what Peter was asking in the first place. I suggest that we have a branch that holds all chipidea patches that are ready for mainline. Otherwise it's really hard to code any new features >> The tree includes my patch we resent yesterday, as Sascha's series >> depends on this. > > Great, I'll take a look. We can (re)post all patches, so that you can identify the patches ready for mainline. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH 0/5] Chipidea driver support for the AR933x platform
On 02/26/2013 03:47 PM, Alexander Shishkin wrote: > Svetoslav Neykov writes: > >> Hi Alex, >> I am working on the incorporating all received comments - thanks to all for >> taking their time to comment. >> Apologies for not replying to the received mails, didn't want to spam with >> OK replies to each separately. > > Great, thanks! > Looks like this patchset will need some synchronization with Sacha's > dr_mode/phy_mode patchset, but I presume you're aware of that. The current version of Sascha's patches is here: http://git.pengutronix.de/?p=mgr/linux.git;a=shortlog;h=refs/heads/chipidea-for-v3.10 Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/2] USB mxs-phy: Register phy with framework
On 02/27/2013 09:10 AM, Felipe Balbi wrote: > On Thu, Feb 14, 2013 at 07:43:54PM +0100, Sascha Hauer wrote: >> On Thu, Feb 14, 2013 at 08:00:11PM +0200, Felipe Balbi wrote: >>> Hi, >>> >>> On Thu, Feb 14, 2013 at 05:23:37PM +0100, Sascha Hauer wrote: >>>> On Thu, Feb 14, 2013 at 12:37:29PM +0200, Felipe Balbi wrote: >>>>> Hi, >>>>> >>>>> On Thu, Jan 31, 2013 at 12:32:16PM +0100, Sascha Hauer wrote: >>>>>> We now have usb_add_phy_dev(), so use it to register with the framework >>>>>> to be able to find the phy from the USB driver. >>>>>> >>>>>> Signed-off-by: Sascha Hauer >>>>> >>>>> Sascha, are you taking this through your tree or you want me to queue it >>>>> up ? >>>> >>>> I would prefer if either you could take it or Alexander Shishkin along >>>> with the other patches in: >>>> >>>> [PATCH v4] USB: add devicetree helpers for determining dr_mode and phy_type >>>> >>>> Could you have a look at them? Alexander is asking for an ack for 1/9, >>>> 2/9, 3/9, 8/9. I just realized you are not on Cc for these. I could >>>> bounce you the patches in case you don't have them in your mailbox. >>> >>> Ok, I'll look at them, no need to bounce. You _do_ realize, however, >>> that Greg has already closed all his trees for v3.9, right ? So anything >>> from now on will be queued for v3.10. >> >> I'm fine with that. I'm just a bit insistent because the chipidea patches are >> floating around for half a year now. Once I have the warm feeling that >> the patches are handled I have time ;) > > I'm getting ready to queue patches but, when looking at this thread, it > seems like another version should've been sent. I'll skip this and look > at the other chipidea threads, I'm currently rebasing Saschas patches to Greg's usb-net. Marc --- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH 5/9] USB: chipidea: add PTW and PTS handling
On 02/14/2013 02:07 PM, Alexander Shishkin wrote: > Sascha Hauer writes: > >> From: Michael Grzeschik >> >> This patch makes it possible to configure the PTW and PTS bits inside >> the portsc register for host and device mode before the driver starts >> and the phy can be addressed as hardware implementation is designed. >> >> Signed-off-by: Michael Grzeschik >> Signed-off-by: Marc Kleine-Budde >> Signed-off-by: Sascha Hauer >> --- >> .../devicetree/bindings/usb/ci13xxx-imx.txt|5 +++ >> drivers/usb/chipidea/bits.h| 14 ++- >> drivers/usb/chipidea/ci13xxx_imx.c |3 ++ >> drivers/usb/chipidea/core.c| 39 >> >> include/linux/usb/chipidea.h |1 + >> 5 files changed, 61 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt >> b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt >> index 5778b9c..dd42ccd 100644 >> --- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt >> +++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt >> @@ -5,6 +5,11 @@ Required properties: >> - reg: Should contain registers location and length >> - interrupts: Should contain controller interrupt >> >> +Recommended properies: >> +- phy_type: the type of the phy connected to the core. Should be one >> + of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this >> + property the PORTSC register won't be touched >> + > > Looks like this bit belongs to patch 3/9, where you're adding devicetree > hooks. Otherwise looks good. Nope. Patch 3/9 adds the device tree helper code. But this patch adds the functionality to the chipidea imx glue code, so the update of the devicetree docs belongs into this patch. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH 6/9] USB chipidea: introduce dual role mode pdata flags
On 02/22/2013 03:09 AM, Peter Chen wrote:
> On Mon, Feb 04, 2013 at 02:24:32PM +0100, Sascha Hauer wrote:
>> Even if a chipidea core is otg capable the board may not. This allows
>> to explicitly set the core to host/peripheral mode. Without these
>> flags the driver falls back to the old behaviour.
>>
>> Signed-off-by: Sascha Hauer
>> ---
>> drivers/usb/chipidea/core.c | 21 +++--
>> include/linux/usb/chipidea.h |2 +-
>> 2 files changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
>> index 04d68cb..c89f2aa 100644
>> --- a/drivers/usb/chipidea/core.c
>> +++ b/drivers/usb/chipidea/core.c
>> @@ -435,6 +435,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>> struct resource *res;
>> void __iomem*base;
>> int ret;
>> +enum usb_dr_mode dr_mode;
>>
>> if (!dev->platform_data) {
>> dev_err(dev, "platform data missing\n");
>> @@ -487,14 +488,22 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>> return -ENODEV;
>> }
>>
>> -ret = ci_hdrc_gadget_init(ci);
>> -if (ret)
>> -dev_info(dev, "doesn't support gadget\n");
>> +if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
>
> Can we change "USB_DR_MODE_PERIPHERAL" to "USB_DR_MODE_GADGET", since we
> always
> use gadget to stands for device or peripheral mode at code (like below
> CI_ROLE_GADGET), it may make code uniform.
Peripheral mode seems to be the more official name compared to gadget. I
vote for keeping peripheral in the DT and changing the chipidea to use
peripheral instead of gadget (in a later patch(es)).
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
[PATCH 7/9] USB chipidea i.MX: introduce dr_mode property
From: Sascha Hauer
The dr_mode devicetree property allows to explicitly specify the
host/peripheral/otg mode. This is necessary for boards without proper
ID pin handling.
Reviewed-by: Peter Chen
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
Documentation/devicetree/bindings/usb/ci13xxx-imx.txt |1 +
drivers/usb/chipidea/ci13xxx_imx.c|1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
index dd42ccd..493a414 100644
--- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
+++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
@@ -9,6 +9,7 @@ Recommended properies:
- phy_type: the type of the phy connected to the core. Should be one
of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this
property the PORTSC register won't be touched
+- dr_mode: One of "host", "peripheral" or "otg". Defaults to "otg"
Optional properties:
- fsl,usbphy: phandler of usb phy that connects to the only one port
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index ebc1148..b598bb8 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -114,6 +114,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
CI13XXX_DISABLE_STREAMING;
pdata->phy_mode = of_usb_get_phy_mode(pdev->dev.of_node);
+ pdata->dr_mode = of_usb_get_dr_mode(pdev->dev.of_node);
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
if (!data) {
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/9] USB chipidea: ci13xxx-imx: create dynamic platformdata
From: Michael Grzeschik
This patch removes the limitation of having only one instance of the
ci13xxx-imx platformdata and makes different configurations possible.
Reviewed-by: Peter Chen
Signed-off-by: Michael Grzeschik
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/chipidea/ci13xxx_imx.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index 8c29122..69024e0 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -85,17 +85,10 @@ EXPORT_SYMBOL_GPL(usbmisc_get_init_data);
/* End of common functions shared by usbmisc drivers*/
-static struct ci13xxx_platform_data ci13xxx_imx_platdata = {
- .name = "ci13xxx_imx",
- .flags = CI13XXX_REQUIRE_TRANSCEIVER |
- CI13XXX_PULLUP_ON_VBUS |
- CI13XXX_DISABLE_STREAMING,
- .capoffset = DEF_CAPOFFSET,
-};
-
static int ci13xxx_imx_probe(struct platform_device *pdev)
{
struct ci13xxx_imx_data *data;
+ struct ci13xxx_platform_data *pdata;
struct platform_device *plat_ci, *phy_pdev;
struct device_node *phy_np;
struct resource *res;
@@ -107,6 +100,18 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
&& !usbmisc_ops)
return -EPROBE_DEFER;
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ dev_err(&pdev->dev, "Failed to allocate CI13xxx-IMX pdata!\n");
+ return -ENOMEM;
+ }
+
+ pdata->name = "ci13xxx_imx";
+ pdata->capoffset = DEF_CAPOFFSET;
+ pdata->flags = CI13XXX_REQUIRE_TRANSCEIVER |
+ CI13XXX_PULLUP_ON_VBUS |
+ CI13XXX_DISABLE_STREAMING;
+
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
if (!data) {
dev_err(&pdev->dev, "Failed to allocate CI13xxx-IMX data!\n");
@@ -168,7 +173,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
reg_vbus = NULL;
}
- ci13xxx_imx_platdata.phy = data->phy;
+ pdata->phy = data->phy;
if (!pdev->dev.dma_mask) {
pdev->dev.dma_mask = devm_kzalloc(&pdev->dev,
@@ -193,7 +198,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
plat_ci = ci13xxx_add_device(&pdev->dev,
pdev->resource, pdev->num_resources,
- &ci13xxx_imx_platdata);
+ pdata);
if (IS_ERR(plat_ci)) {
ret = PTR_ERR(plat_ci);
dev_err(&pdev->dev,
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/9] USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy
From: Sascha Hauer
This patch replaces the hand crafted code to retrieve the phy's phandle from
the DT by the helper function devm_usb_get_phy_by_phandle() which has been
added in commit:
"5d3c28b usb: otg: add device tree support to otg library"
Reviewed-by: Kishon Vijay Abraham I
Reviewed-by: Peter Chen
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/chipidea/ci13xxx_imx.c | 38
1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index b598bb8..6e720ce 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -30,7 +30,6 @@
((struct usb_phy *)platform_get_drvdata(pdev))
struct ci13xxx_imx_data {
- struct device_node *phy_np;
struct usb_phy *phy;
struct platform_device *ci_pdev;
struct clk *clk;
@@ -90,12 +89,12 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
{
struct ci13xxx_imx_data *data;
struct ci13xxx_platform_data *pdata;
- struct platform_device *plat_ci, *phy_pdev;
- struct device_node *phy_np;
+ struct platform_device *plat_ci;
struct resource *res;
struct regulator *reg_vbus;
struct pinctrl *pinctrl;
int ret;
+ struct usb_phy *phy;
if (of_find_property(pdev->dev.of_node, "fsl,usbmisc", NULL)
&& !usbmisc_ops)
@@ -147,19 +146,20 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
return ret;
}
- phy_np = of_parse_phandle(pdev->dev.of_node, "fsl,usbphy", 0);
- if (phy_np) {
- data->phy_np = phy_np;
- phy_pdev = of_find_device_by_node(phy_np);
- if (phy_pdev) {
- struct usb_phy *phy;
- phy = pdev_to_phy(phy_pdev);
- if (phy &&
- try_module_get(phy_pdev->dev.driver->owner)) {
- usb_phy_init(phy);
- data->phy = phy;
- }
+ phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0);
+ if (PTR_ERR(phy) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto err_clk;
+ }
+
+ if (!IS_ERR(phy)) {
+ ret = usb_phy_init(phy);
+ if (ret) {
+ dev_err(&pdev->dev, "unable to init phy: %d\n", ret);
+ goto err_clk;
}
+
+ data->phy = phy;
}
/* we only support host now, so enable vbus here */
@@ -170,7 +170,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
dev_err(&pdev->dev,
"Failed to enable vbus regulator, err=%d\n",
ret);
- goto put_np;
+ goto err_clk;
}
data->reg_vbus = reg_vbus;
} else {
@@ -222,9 +222,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
err:
if (reg_vbus)
regulator_disable(reg_vbus);
-put_np:
- if (phy_np)
- of_node_put(phy_np);
+err_clk:
clk_disable_unprepare(data->clk);
return ret;
}
@@ -244,8 +242,6 @@ static int ci13xxx_imx_remove(struct platform_device *pdev)
module_put(data->phy->dev->driver->owner);
}
- of_node_put(data->phy_np);
-
clk_disable_unprepare(data->clk);
platform_set_drvdata(pdev, NULL);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/9] USB chipidea: add PTW and PTS handling
From: Michael Grzeschik
This patch makes it possible to configure the PTW and PTS bits inside
the portsc register for host and device mode before the driver starts
and the phy can be addressed as hardware implementation is designed.
Signed-off-by: Michael Grzeschik
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
.../devicetree/bindings/usb/ci13xxx-imx.txt|5 +++
drivers/usb/chipidea/bits.h| 14 ++-
drivers/usb/chipidea/ci13xxx_imx.c |3 ++
drivers/usb/chipidea/core.c| 39
include/linux/usb/chipidea.h |1 +
5 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
index 5778b9c..dd42ccd 100644
--- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
+++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
@@ -5,6 +5,11 @@ Required properties:
- reg: Should contain registers location and length
- interrupts: Should contain controller interrupt
+Recommended properies:
+- phy_type: the type of the phy connected to the core. Should be one
+ of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this
+ property the PORTSC register won't be touched
+
Optional properties:
- fsl,usbphy: phandler of usb phy that connects to the only one port
- fsl,usbmisc: phandler of non-core register device, with one argument
diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
index 050de85..d8ffc2f 100644
--- a/drivers/usb/chipidea/bits.h
+++ b/drivers/usb/chipidea/bits.h
@@ -48,10 +48,22 @@
#define PORTSC_SUSP BIT(7)
#define PORTSC_HSPBIT(9)
#define PORTSC_PTC(0x0FUL << 16)
+/* PTS and PTW for non lpm version only */
+#define PORTSC_PTS(d) d) & 0x3) << 30) | (((d) & 0x4) ? BIT(25) :
0))
+#define PORTSC_PTWBIT(28)
/* DEVLC */
#define DEVLC_PSPD(0x03UL << 25)
-#defineDEVLC_PSPD_HS (0x02UL << 25)
+#define DEVLC_PSPD_HS (0x02UL << 25)
+#define DEVLC_PTW BIT(27)
+#define DEVLC_STS BIT(28)
+#define DEVLC_PTS(d) (((d) & 0x7) << 29)
+
+/* Encoding for DEVLC_PTS and PORTSC_PTS */
+#define PTS_UTMI 0
+#define PTS_ULPI 2
+#define PTS_SERIAL3
+#define PTS_HSIC 4
/* OTGSC */
#define OTGSC_IDPU BIT(5)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index 69024e0..ebc1148 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include "ci.h"
#include "ci13xxx_imx.h"
@@ -112,6 +113,8 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
CI13XXX_PULLUP_ON_VBUS |
CI13XXX_DISABLE_STREAMING;
+ pdata->phy_mode = of_usb_get_phy_mode(pdev->dev.of_node);
+
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
if (!data) {
dev_err(&pdev->dev, "Failed to allocate CI13xxx-IMX data!\n");
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 57cae1f..04d68cb 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -67,6 +67,8 @@
#include
#include
#include
+#include
+#include
#include "ci.h"
#include "udc.h"
@@ -211,6 +213,41 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem
*base)
return 0;
}
+static void hw_phymode_configure(struct ci13xxx *ci)
+{
+ u32 portsc, lpm;
+
+ switch (ci->platdata->phy_mode) {
+ case USBPHY_INTERFACE_MODE_UTMI:
+ portsc = PORTSC_PTS(PTS_UTMI);
+ lpm = DEVLC_PTS(PTS_UTMI);
+ break;
+ case USBPHY_INTERFACE_MODE_UTMIW:
+ portsc = PORTSC_PTS(PTS_UTMI) | PORTSC_PTW;
+ lpm = DEVLC_PTS(PTS_UTMI) | DEVLC_PTW;
+ break;
+ case USBPHY_INTERFACE_MODE_ULPI:
+ portsc = PORTSC_PTS(PTS_ULPI);
+ lpm = DEVLC_PTS(PTS_ULPI);
+ break;
+ case USBPHY_INTERFACE_MODE_SERIAL:
+ portsc = PORTSC_PTS(PTS_SERIAL);
+ lpm = DEVLC_PTS(PTS_SERIAL);
+ break;
+ case USBPHY_INTERFACE_MODE_HSIC:
+ portsc = PORTSC_PTS(PTS_HSIC);
+ lpm = DEVLC_PTS(PTS_HSIC);
+ break;
+ default:
+ return;
+ }
+
+ if (ci->hw_bank.lpm)
+ hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
+ else
+ hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW
[PATCH 8/9] USB mxs-phy: Register phy with framework
From: Sascha Hauer
We now have usb_add_phy_dev(), so use it to register with the framework
to be able to find the phy from the USB driver.
Reviewed-by: Kishon Vijay Abraham I
Reviewed-by: Peter Chen
Acked-by: Felipe Balbi
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/otg/mxs-phy.c |9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
index b0d9f11..aa40325 100644
--- a/drivers/usb/otg/mxs-phy.c
+++ b/drivers/usb/otg/mxs-phy.c
@@ -127,6 +127,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
void __iomem *base;
struct clk *clk;
struct mxs_phy *mxs_phy;
+ int ret;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -166,11 +167,19 @@ static int mxs_phy_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, &mxs_phy->phy);
+ ret = usb_add_phy_dev(&mxs_phy->phy);
+ if (ret)
+ return ret;
+
return 0;
}
static int mxs_phy_remove(struct platform_device *pdev)
{
+ struct mxs_phy *mxs_phy = platform_get_drvdata(pdev);
+
+ usb_remove_phy(&mxs_phy->phy);
+
platform_set_drvdata(pdev, NULL);
return 0;
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/9] USB chipidea: introduce dual role mode pdata flags
From: Sascha Hauer
Even if a chipidea core is otg capable the board may not. This allows
to explicitly set the core to host/peripheral mode. Without these
flags the driver falls back to the old behaviour.
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/chipidea/core.c | 22 --
include/linux/usb/chipidea.h |2 +-
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 04d68cb..ec27060 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -435,6 +435,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
struct resource *res;
void __iomem*base;
int ret;
+ enum usb_dr_mode dr_mode;
if (!dev->platform_data) {
dev_err(dev, "platform data missing\n");
@@ -487,14 +488,23 @@ static int ci_hdrc_probe(struct platform_device *pdev)
return -ENODEV;
}
+ /* For now we treat dual-role as otg */
+ dr_mode = ci->platdata->dr_mode;
+ if (dr_mode == USB_DR_MODE_UNKNOWN || dr_mode == USB_DR_MODE_DUAL_ROLE)
+ dr_mode = USB_DR_MODE_OTG;
+
/* initialize role(s) before the interrupt is requested */
- ret = ci_hdrc_host_init(ci);
- if (ret)
- dev_info(dev, "doesn't support host\n");
+ if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
+ ret = ci_hdrc_host_init(ci);
+ if (ret)
+ dev_info(dev, "doesn't support host\n");
+ }
- ret = ci_hdrc_gadget_init(ci);
- if (ret)
- dev_info(dev, "doesn't support gadget\n");
+ if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
+ ret = ci_hdrc_gadget_init(ci);
+ if (ret)
+ dev_info(dev, "doesn't support gadget\n");
+ }
if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
dev_err(dev, "no supported roles\n");
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 1a2aa18..b314647 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -20,7 +20,7 @@ struct ci13xxx_platform_data {
#define CI13XXX_REQUIRE_TRANSCEIVERBIT(1)
#define CI13XXX_PULLUP_ON_VBUS BIT(2)
#define CI13XXX_DISABLE_STREAMING BIT(3)
-
+ enum usb_dr_modedr_mode;
#define CI13XXX_CONTROLLER_RESET_EVENT 0
#define CI13XXX_CONTROLLER_STOPPED_EVENT 1
void(*notify_event) (struct ci13xxx *ci, unsigned event);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/9] USB otg: use try_module_get in all usb_get_phy functions and add missing module_put
In patch "5d3c28b usb: otg: add device tree support to otg library"
devm_usb_get_phy_by_phandle() was added. It uses try_module_get() to lock the
phy driver in memory. The corresponding module_put() is missing in that patch.
This patch adds try_module_get() to usb_get_phy() and usb_get_phy_dev().
Further the missing module_put() is added to usb_put_phy().
Reviewed-by: Kishon Vijay Abraham I
Acked-by: Felipe Balbi
Signed-off-by: Marc Kleine-Budde
Signed-off-by: Michael Grzeschik
---
drivers/usb/otg/otg.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
index e181439..2bd03d2 100644
--- a/drivers/usb/otg/otg.c
+++ b/drivers/usb/otg/otg.c
@@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
spin_lock_irqsave(&phy_lock, flags);
phy = __usb_find_phy(&phy_list, type);
- if (IS_ERR(phy)) {
+ if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
pr_err("unable to find transceiver of type %s\n",
usb_phy_type_string(type));
goto err0;
@@ -228,7 +228,7 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8
index)
spin_lock_irqsave(&phy_lock, flags);
phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
- if (IS_ERR(phy)) {
+ if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
pr_err("unable to find transceiver\n");
goto err0;
}
@@ -301,8 +301,12 @@ EXPORT_SYMBOL(devm_usb_put_phy);
*/
void usb_put_phy(struct usb_phy *x)
{
- if (x)
+ if (x) {
+ struct module *owner = x->dev->driver->owner;
+
put_device(x->dev);
+ module_put(owner);
+ }
}
EXPORT_SYMBOL(usb_put_phy);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/9] USB: move bulk of otg/otg.c to phy/phy.c
From: Sascha Hauer
Most of otg/otg.c is not otg specific, but phy specific, so move it
to the phy directory.
Cc: Felipe Balbi
Reported-by: Kishon Vijay Abraham I
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/otg/otg.c| 427
drivers/usb/phy/Makefile |1 +
drivers/usb/phy/phy.c| 438 ++
3 files changed, 439 insertions(+), 427 deletions(-)
create mode 100644 drivers/usb/phy/phy.c
diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
index 2bd03d2..358cfd9 100644
--- a/drivers/usb/otg/otg.c
+++ b/drivers/usb/otg/otg.c
@@ -8,436 +8,9 @@
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*/
-
-#include
#include
-#include
-#include
-#include
-#include
-#include
-
#include
-static LIST_HEAD(phy_list);
-static LIST_HEAD(phy_bind_list);
-static DEFINE_SPINLOCK(phy_lock);
-
-static struct usb_phy *__usb_find_phy(struct list_head *list,
- enum usb_phy_type type)
-{
- struct usb_phy *phy = NULL;
-
- list_for_each_entry(phy, list, head) {
- if (phy->type != type)
- continue;
-
- return phy;
- }
-
- return ERR_PTR(-ENODEV);
-}
-
-static struct usb_phy *__usb_find_phy_dev(struct device *dev,
- struct list_head *list, u8 index)
-{
- struct usb_phy_bind *phy_bind = NULL;
-
- list_for_each_entry(phy_bind, list, list) {
- if (!(strcmp(phy_bind->dev_name, dev_name(dev))) &&
- phy_bind->index == index) {
- if (phy_bind->phy)
- return phy_bind->phy;
- else
- return ERR_PTR(-EPROBE_DEFER);
- }
- }
-
- return ERR_PTR(-ENODEV);
-}
-
-static struct usb_phy *__of_usb_find_phy(struct device_node *node)
-{
- struct usb_phy *phy;
-
- list_for_each_entry(phy, &phy_list, head) {
- if (node != phy->dev->of_node)
- continue;
-
- return phy;
- }
-
- return ERR_PTR(-ENODEV);
-}
-
-static void devm_usb_phy_release(struct device *dev, void *res)
-{
- struct usb_phy *phy = *(struct usb_phy **)res;
-
- usb_put_phy(phy);
-}
-
-static int devm_usb_phy_match(struct device *dev, void *res, void *match_data)
-{
- return res == match_data;
-}
-
-/**
- * devm_usb_get_phy - find the USB PHY
- * @dev - device that requests this phy
- * @type - the type of the phy the controller requires
- *
- * Gets the phy using usb_get_phy(), and associates a device with it using
- * devres. On driver detach, release function is invoked on the devres data,
- * then, devres data is freed.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *devm_usb_get_phy(struct device *dev, enum usb_phy_type type)
-{
- struct usb_phy **ptr, *phy;
-
- ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
- if (!ptr)
- return NULL;
-
- phy = usb_get_phy(type);
- if (!IS_ERR(phy)) {
- *ptr = phy;
- devres_add(dev, ptr);
- } else
- devres_free(ptr);
-
- return phy;
-}
-EXPORT_SYMBOL(devm_usb_get_phy);
-
-/**
- * usb_get_phy - find the USB PHY
- * @type - the type of the phy the controller requires
- *
- * Returns the phy driver, after getting a refcount to it; or
- * -ENODEV if there is no such phy. The caller is responsible for
- * calling usb_put_phy() to release that count.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *usb_get_phy(enum usb_phy_type type)
-{
- struct usb_phy *phy = NULL;
- unsigned long flags;
-
- spin_lock_irqsave(&phy_lock, flags);
-
- phy = __usb_find_phy(&phy_list, type);
- if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
- pr_err("unable to find transceiver of type %s\n",
- usb_phy_type_string(type));
- goto err0;
- }
-
- get_device(phy->dev);
-
-err0:
- spin_unlock_irqrestore(&phy_lock, flags);
-
- return phy;
-}
-EXPORT_SYMBOL(usb_get_phy);
-
- /**
- * devm_usb_get_phy_by_phandle - find the USB PHY by phandle
- * @dev - device that requests this phy
- * @phandle - name of the property holding the phy phandle value
- * @index - the index of the phy
- *
- * Returns the phy driver associated with the given phandle value,
- * after getting a refcount to it, -ENODEV if there is no such phy or
- * -EPROBE_DEFER if there is a phandle to the phy, but the device is
- * not yet loaded. While at that, it also associates the device with
- * the phy using devres. On driver detach, release function is invoked
- * on the devres data, then, devre
[PATCH v5 0/9] USB: add devicetree helpers for determining dr_mode and phy_type
Hello, this is the 5th round of Sascha's patch series, rebased to Greg's usb-next. Please consider to apply. regards, Marc changes since v4: - add support for USB_DR_MODE_DUAL_ROLE in dt helper changes since v3: - add phy patches (which were accidently already part of v2) - Use OP_DEVLC instead of OP_PORTSC for lpm case - Use enum usb_dr_mode ub ci_hdrc_probe() changes since v2: - fix adding of GPL Header was in wrong patch - add missing hunk for new file of.c changes since v1: - move phy specific of helper to drivers/usb/phy/of.c - use strcmp instead of strcasecmp for matching property values - change usb_phy_dr_mode to usb_dr_mode - change USBPHY_INTERFACE_MODE_NA to USBPHY_INTERFACE_MODE_UNKNOWN - add copyright header to new files - chipidea: drop mdelay at end of PTS/PTW setup - chipidea: implement lpm core type handling for PTS/PTW The following changes since commit 74e1a2a39355b2d3ae8c60c78d8add162c6d7183: Merge tag 'usb-3.9-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb (2013-02-21 12:20:00 -0800) are available in the git repository at: git://git.pengutronix.de/git/mkl/linux.git tags/usb-chipidea-for-next-v5 for you to fetch changes up to 1065bb064c06565cc0b86337d52977ab5afc3e90: USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy (2013-02-27 12:54:13 +0100) USB: chipidea patches for v3.10 These add OF helpers for handling the dr_mode and phy_type property and makes use of them in the chipidea driver. -------- Marc Kleine-Budde (1): USB otg: use try_module_get in all usb_get_phy functions and add missing module_put Michael Grzeschik (3): USB: add devicetree helpers for determining dr_mode and phy_type USB chipidea: ci13xxx-imx: create dynamic platformdata USB chipidea: add PTW and PTS handling Sascha Hauer (5): USB: move bulk of otg/otg.c to phy/phy.c USB chipidea: introduce dual role mode pdata flags USB chipidea i.MX: introduce dr_mode property USB mxs-phy: Register phy with framework USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy .../devicetree/bindings/usb/ci13xxx-imx.txt|6 + drivers/usb/chipidea/bits.h| 14 +- drivers/usb/chipidea/ci13xxx_imx.c | 67 +-- drivers/usb/chipidea/core.c| 61 ++- drivers/usb/otg/mxs-phy.c |9 + drivers/usb/otg/otg.c | 423 --- drivers/usb/phy/Makefile |2 + drivers/usb/phy/of.c | 47 +++ drivers/usb/phy/phy.c | 438 drivers/usb/usb-common.c | 37 ++ include/linux/usb/chipidea.h |3 +- include/linux/usb/of.h | 27 ++ include/linux/usb/otg.h|8 + include/linux/usb/phy.h|9 + 14 files changed, 689 insertions(+), 462 deletions(-) create mode 100644 drivers/usb/phy/of.c create mode 100644 drivers/usb/phy/phy.c create mode 100644 include/linux/usb/of.h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type
From: Michael Grzeschik
This adds two little devicetree helper functions for determining the dr_mode
(host, peripheral, otg, dual-role) and phy_type (utmi, ulpi,...) from the
devicetree.
Cc: Felipe Balbi
Signed-off-by: Michael Grzeschik
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/phy/Makefile |1 +
drivers/usb/phy/of.c | 47 ++
drivers/usb/usb-common.c | 37
include/linux/usb/of.h | 27 ++
include/linux/usb/otg.h |8
include/linux/usb/phy.h |9 +
6 files changed, 129 insertions(+)
create mode 100644 drivers/usb/phy/of.c
create mode 100644 include/linux/usb/of.h
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 9fa6327..e1be1e8 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -5,6 +5,7 @@
ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
obj-$(CONFIG_USB_OTG_UTILS)+= phy.o
+obj-$(CONFIG_OF) += of.o
obj-$(CONFIG_OMAP_USB2)+= omap-usb2.o
obj-$(CONFIG_OMAP_USB3)+= omap-usb3.o
obj-$(CONFIG_OMAP_CONTROL_USB) += omap-control-usb.o
diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c
new file mode 100644
index 000..e6f3b74
--- /dev/null
+++ b/drivers/usb/phy/of.c
@@ -0,0 +1,47 @@
+/*
+ * USB of helper code
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+static const char *usbphy_modes[] = {
+ [USBPHY_INTERFACE_MODE_UNKNOWN] = "",
+ [USBPHY_INTERFACE_MODE_UTMI]= "utmi",
+ [USBPHY_INTERFACE_MODE_UTMIW] = "utmi_wide",
+ [USBPHY_INTERFACE_MODE_ULPI]= "ulpi",
+ [USBPHY_INTERFACE_MODE_SERIAL] = "serial",
+ [USBPHY_INTERFACE_MODE_HSIC]= "hsic",
+};
+
+/**
+ * of_usb_get_phy_mode - Get phy mode for given device_node
+ * @np:Pointer to the given device_node
+ *
+ * The function gets phy interface string from property 'phy_type',
+ * and returns the correspondig enum usb_phy_interface
+ */
+enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np)
+{
+ const char *phy_type;
+ int err, i;
+
+ err = of_property_read_string(np, "phy_type", &phy_type);
+ if (err < 0)
+ return USBPHY_INTERFACE_MODE_UNKNOWN;
+
+ for (i = 0; i < ARRAY_SIZE(usbphy_modes); i++)
+ if (!strcmp(phy_type, usbphy_modes[i]))
+ return i;
+
+ return USBPHY_INTERFACE_MODE_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_phy_mode);
diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
index d29503e..18b994b 100644
--- a/drivers/usb/usb-common.c
+++ b/drivers/usb/usb-common.c
@@ -14,6 +14,9 @@
#include
#include
#include
+#include
+#include
+#include
const char *usb_speed_string(enum usb_device_speed speed)
{
@@ -32,4 +35,38 @@ const char *usb_speed_string(enum usb_device_speed speed)
}
EXPORT_SYMBOL_GPL(usb_speed_string);
+#ifdef CONFIG_OF
+static const char *usb_dr_modes[] = {
+ [USB_DR_MODE_UNKNOWN] = "",
+ [USB_DR_MODE_HOST] = "host",
+ [USB_DR_MODE_PERIPHERAL]= "peripheral",
+ [USB_DR_MODE_OTG] = "otg",
+ [USB_DR_MODE_DUAL_ROLE] = "dual-role",
+};
+
+/**
+ * of_usb_get_dr_mode - Get dual role mode for given device_node
+ * @np:Pointer to the given device_node
+ *
+ * The function gets phy interface string from property 'dr_mode',
+ * and returns the correspondig enum usb_dr_mode
+ */
+enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
+{
+ const char *dr_mode;
+ int err, i;
+
+ err = of_property_read_string(np, "dr_mode", &dr_mode);
+ if (err < 0)
+ return USB_DR_MODE_UNKNOWN;
+
+ for (i = 0; i < ARRAY_SIZE(usb_dr_modes); i++)
+ if (!strcmp(dr_mode, usb_dr_modes[i]))
+ return i;
+
+ return USB_DR_MODE_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_dr_mode);
+#endif
+
MODULE_LICENSE("GPL");
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
new file mode 100644
index 000..4681a20
--- /dev/null
+++ b/include/linux/usb/of.h
@@ -0,0 +1,27 @@
+/*
+ * OF helpers for usb devices.
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_USB_OF_H
+#define __LINUX_USB_OF_H
+
+#include
+
+#ifdef CONFIG_OF
+enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np);
+enum usb_dr_mode of_usb_get_dr_mode(str
Re: [PATCH 2/9] USB: move bulk of otg/otg.c to phy/phy.c
On 02/27/2013 01:27 PM, Marc Kleine-Budde wrote: > From: Sascha Hauer > > Most of otg/otg.c is not otg specific, but phy specific, so move it > to the phy directory. > > Cc: Felipe Balbi > Reported-by: Kishon Vijay Abraham I > Signed-off-by: Sascha Hauer > Signed-off-by: Marc Kleine-Budde Felipe, can you please review this patch and give us your Acked-by? regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type
On 02/27/2013 01:27 PM, Marc Kleine-Budde wrote: > From: Michael Grzeschik > > This adds two little devicetree helper functions for determining the dr_mode > (host, peripheral, otg, dual-role) and phy_type (utmi, ulpi,...) from the > devicetree. > > Cc: Felipe Balbi > Signed-off-by: Michael Grzeschik > Signed-off-by: Sascha Hauer > Signed-off-by: Marc Kleine-Budde Felipe, same here. Your Acked-by is appreciated. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type
On 02/27/2013 01:32 PM, Felipe Balbi wrote: > On Wed, Feb 27, 2013 at 01:27:09PM +0100, Marc Kleine-Budde wrote: >> From: Michael Grzeschik >> >> This adds two little devicetree helper functions for determining the dr_mode > > it looks to me that "mode" is enough. s/dr_mode/mode/ There already is that DT binding, we just created a helper function for that. git grep -e dr_mode\ *: linus/master Documentation/ linus/master:Documentation/devicetree/bindings/usb/fsl-usb.txt: - dr_mode : indicates the working mode for "fsl-usb2-dr" compatible linus/master:Documentation/devicetree/bindings/usb/nvidia,tegra20-ehci.txt: - dr_mode : dual role mode. Indicates the working mode for Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/9] USB otg: use try_module_get in all usb_get_phy functions and add missing module_put
On 02/27/2013 01:31 PM, Felipe Balbi wrote: > On Wed, Feb 27, 2013 at 01:27:07PM +0100, Marc Kleine-Budde wrote: >> In patch "5d3c28b usb: otg: add device tree support to otg library" >> devm_usb_get_phy_by_phandle() was added. It uses try_module_get() to lock the >> phy driver in memory. The corresponding module_put() is missing in that >> patch. >> >> This patch adds try_module_get() to usb_get_phy() and usb_get_phy_dev(). >> Further the missing module_put() is added to usb_put_phy(). >> >> Reviewed-by: Kishon Vijay Abraham I >> Acked-by: Felipe Balbi >> Signed-off-by: Marc Kleine-Budde >> Signed-off-by: Michael Grzeschik > > this one doesn't apply to my tree. What did you use as a base ? As stated in the cover letter: Greg's usb-next. git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git I'd really appreciate if this series goes into a single tree. So that other chipidea developers can base their work on this. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/9] USB otg: use try_module_get in all usb_get_phy functions and add missing module_put
On 02/27/2013 01:43 PM, Felipe Balbi wrote: > Hi, > > On Wed, Feb 27, 2013 at 01:40:51PM +0100, Marc Kleine-Budde wrote: >> On 02/27/2013 01:31 PM, Felipe Balbi wrote: >>> On Wed, Feb 27, 2013 at 01:27:07PM +0100, Marc Kleine-Budde wrote: >>>> In patch "5d3c28b usb: otg: add device tree support to otg library" >>>> devm_usb_get_phy_by_phandle() was added. It uses try_module_get() to lock >>>> the >>>> phy driver in memory. The corresponding module_put() is missing in that >>>> patch. >>>> >>>> This patch adds try_module_get() to usb_get_phy() and usb_get_phy_dev(). >>>> Further the missing module_put() is added to usb_put_phy(). >>>> >>>> Reviewed-by: Kishon Vijay Abraham I >>>> Acked-by: Felipe Balbi >>>> Signed-off-by: Marc Kleine-Budde >>>> Signed-off-by: Michael Grzeschik >>> >>> this one doesn't apply to my tree. What did you use as a base ? >> >> As stated in the cover letter: Greg's usb-next. >> >> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git >> >> I'd really appreciate if this series goes into a single tree. So that >> other chipidea developers can base their work on this. > > it eventually will all go through Greg's queue, but drivers/usb/phy and > drivers/usb/otg/ needs to go through my tree so we avoid conflicts > later, sorry. Oh - this is going to be complicated. I'll rip the series into 3 parts and repost. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/9] USB otg: use try_module_get in all usb_get_phy functions and add missing module_put
On 02/27/2013 01:47 PM, Felipe Balbi wrote: > On Wed, Feb 27, 2013 at 01:46:21PM +0100, Marc Kleine-Budde wrote: >> On 02/27/2013 01:43 PM, Felipe Balbi wrote: >>> Hi, >>> >>> On Wed, Feb 27, 2013 at 01:40:51PM +0100, Marc Kleine-Budde wrote: >>>> On 02/27/2013 01:31 PM, Felipe Balbi wrote: >>>>> On Wed, Feb 27, 2013 at 01:27:07PM +0100, Marc Kleine-Budde wrote: >>>>>> In patch "5d3c28b usb: otg: add device tree support to otg library" >>>>>> devm_usb_get_phy_by_phandle() was added. It uses try_module_get() to >>>>>> lock the >>>>>> phy driver in memory. The corresponding module_put() is missing in that >>>>>> patch. >>>>>> >>>>>> This patch adds try_module_get() to usb_get_phy() and usb_get_phy_dev(). >>>>>> Further the missing module_put() is added to usb_put_phy(). >>>>>> >>>>>> Reviewed-by: Kishon Vijay Abraham I >>>>>> Acked-by: Felipe Balbi >>>>>> Signed-off-by: Marc Kleine-Budde >>>>>> Signed-off-by: Michael Grzeschik >>>>> >>>>> this one doesn't apply to my tree. What did you use as a base ? >>>> >>>> As stated in the cover letter: Greg's usb-next. >>>> >>>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git >>>> >>>> I'd really appreciate if this series goes into a single tree. So that >>>> other chipidea developers can base their work on this. >>> >>> it eventually will all go through Greg's queue, but drivers/usb/phy and >>> drivers/usb/otg/ needs to go through my tree so we avoid conflicts >>> later, sorry. >> >> Oh - this is going to be complicated. I'll rip the series into 3 parts >> and repost. > > let's try like this: > > drivers/usb/otg and drivers/usb/phy which don't create dependencies for > chipidea and other patches which create dependencies. > > Then we can figure out how to handle the dependencies. > That'll be these 3 branches then: otg-for-v3.9 (bugfix for v3.9): 6bef020 USB otg: use try_module_get in all usb_get_phy functions and add missing module_put otg-for-v3.10 (depends on otg-for-v3.9): a0e17f5 USB: move bulk of otg/otg.c to phy/phy.c a6fc0d1 USB: add devicetree helpers for determining dr_mode and phy_type 8a4c9f8 USB mxs-phy: Register phy with framework chipidea-for-v3.10 (most patches depend on otg-for-v3.10): 15c930e USB chipidea: ci13xxx-imx: create dynamic platformdata 9466e85 USB chipidea: add PTW and PTS handling ad2cc0d USB chipidea: introduce dual role mode pdata flags 8e7f1bb USB chipidea i.MX: introduce dr_mode property 5d83722 USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/9] USB otg: use try_module_get in all usb_get_phy functions and add missing module_put
On 02/27/2013 02:42 PM, Felipe Balbi wrote: >> That'll be these 3 branches then: >> >> otg-for-v3.9 (bugfix for v3.9): >> 6bef020 USB otg: use try_module_get in all usb_get_phy functions and add >> missing module_put >> >> otg-for-v3.10 (depends on otg-for-v3.9): >> a0e17f5 USB: move bulk of otg/otg.c to phy/phy.c >> a6fc0d1 USB: add devicetree helpers for determining dr_mode and phy_type >> 8a4c9f8 USB mxs-phy: Register phy with framework >> >> chipidea-for-v3.10 (most patches depend on otg-for-v3.10): >> 15c930e USB chipidea: ci13xxx-imx: create dynamic platformdata >> 9466e85 USB chipidea: add PTW and PTS handling >> ad2cc0d USB chipidea: introduce dual role mode pdata flags >> 8e7f1bb USB chipidea i.MX: introduce dr_mode property >> 5d83722 USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy > > sounds good to me, please make sure to always split your series like > this. You can't expect me (or any maintainer for that matter) to pick > patches to proper branches based on your commit logs alone. > > remeber that every single maintainer needs to handle hundreds of emails > a week (if not a day) and if patches are properly split and cleanly > mention if it's a bugfix or not, things will go wrong. That was just a proposal. You'll find these series in your inbox in a few minutes. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
[PATCH] USB otg: use try_module_get in all usb_get_phy functions and add missing module_put
In patch "5d3c28b usb: otg: add device tree support to otg library"
devm_usb_get_phy_by_phandle() was added. It uses try_module_get() to lock the
phy driver in memory. The corresponding module_put() is missing in that patch.
This patch adds try_module_get() to usb_get_phy() and usb_get_phy_dev().
Further the missing module_put() is added to usb_put_phy().
Reviewed-by: Kishon Vijay Abraham I
Acked-by: Felipe Balbi
Signed-off-by: Marc Kleine-Budde
Signed-off-by: Michael Grzeschik
---
drivers/usb/otg/otg.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
index e181439..2bd03d2 100644
--- a/drivers/usb/otg/otg.c
+++ b/drivers/usb/otg/otg.c
@@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
spin_lock_irqsave(&phy_lock, flags);
phy = __usb_find_phy(&phy_list, type);
- if (IS_ERR(phy)) {
+ if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
pr_err("unable to find transceiver of type %s\n",
usb_phy_type_string(type));
goto err0;
@@ -228,7 +228,7 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8
index)
spin_lock_irqsave(&phy_lock, flags);
phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
- if (IS_ERR(phy)) {
+ if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
pr_err("unable to find transceiver\n");
goto err0;
}
@@ -301,8 +301,12 @@ EXPORT_SYMBOL(devm_usb_put_phy);
*/
void usb_put_phy(struct usb_phy *x)
{
- if (x)
+ if (x) {
+ struct module *owner = x->dev->driver->owner;
+
put_device(x->dev);
+ module_put(owner);
+ }
}
EXPORT_SYMBOL(usb_put_phy);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] otg-for-v3.9-v1: USB otg: fix usage of try_module_get() and module_put()
Hello, this patch is intended for v3.9 and applies to Greg's usb/master tree. If fixes the use of try_module_get() and module_put() in all usb_get_phy functions. regards, Marc The following changes since commit 74e1a2a39355b2d3ae8c60c78d8add162c6d7183: Merge tag 'usb-3.9-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb (2013-02-21 12:20:00 -0800) are available in the git repository at: git://git.pengutronix.de/git/mkl/linux.git tags/otg-for-v3.9-v1 for you to fetch changes up to 6bef020b4aebd7886281fb7fb37c788d5a365eea: USB otg: use try_module_get in all usb_get_phy functions and add missing module_put (2013-02-27 12:53:15 +0100) USB otg: add missing try_module_get and module_put Add try_module_get() to usb_get_phy() and usb_get_phy_dev(). Further the missing module_put() is added to usb_put_phy() -------- Marc Kleine-Budde (1): USB otg: use try_module_get in all usb_get_phy functions and add missing module_put drivers/usb/otg/otg.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] otg-for-v3.10-v1: separate phy code and add DT helper
Hello, this series depends on the bugfix patch "USB otg: use try_module_get in all usb_get_phy functions and add missing module_put" (a.k.a tags/otg-for-v3.9-v1) posted earlier and is inteded for v3.10. It separates the phy from the otg code and adds DT helper functions. Further mxs-phy uses the new usb_add_phy_dev() function to register it's phy. regards, Marc The following changes since commit 6bef020b4aebd7886281fb7fb37c788d5a365eea: USB otg: use try_module_get in all usb_get_phy functions and add missing module_put (2013-02-27 12:53:15 +0100) are available in the git repository at: git://git.pengutronix.de/git/mkl/linux.git tags/otg-for-v3.10-v1 for you to fetch changes up to 8a4c9f8489b2fce9e7bf0eb43fdb160ab51adc2c: USB mxs-phy: Register phy with framework (2013-02-27 13:48:21 +0100) USB otg, phy: separate phy and add DT helper Move phy related code into separate file and add device tree helper functions. Michael Grzeschik (1): USB: add devicetree helpers for determining dr_mode and phy_type Sascha Hauer (2): USB: move bulk of otg/otg.c to phy/phy.c USB mxs-phy: Register phy with framework drivers/usb/otg/mxs-phy.c |9 + drivers/usb/otg/otg.c | 427 --- drivers/usb/phy/Makefile |2 + drivers/usb/phy/of.c | 47 + drivers/usb/phy/phy.c | 438 + drivers/usb/usb-common.c | 37 include/linux/usb/of.h| 27 +++ include/linux/usb/otg.h |8 + include/linux/usb/phy.h |9 + 9 files changed, 577 insertions(+), 427 deletions(-) create mode 100644 drivers/usb/phy/of.c create mode 100644 drivers/usb/phy/phy.c create mode 100644 include/linux/usb/of.h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] USB: add devicetree helpers for determining dr_mode and phy_type
From: Michael Grzeschik
This adds two little devicetree helper functions for determining the dr_mode
(host, peripheral, otg, dual-role) and phy_type (utmi, ulpi,...) from the
devicetree.
Cc: Felipe Balbi
Signed-off-by: Michael Grzeschik
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/phy/Makefile |1 +
drivers/usb/phy/of.c | 47 ++
drivers/usb/usb-common.c | 37
include/linux/usb/of.h | 27 ++
include/linux/usb/otg.h |8
include/linux/usb/phy.h |9 +
6 files changed, 129 insertions(+)
create mode 100644 drivers/usb/phy/of.c
create mode 100644 include/linux/usb/of.h
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 9fa6327..e1be1e8 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -5,6 +5,7 @@
ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
obj-$(CONFIG_USB_OTG_UTILS)+= phy.o
+obj-$(CONFIG_OF) += of.o
obj-$(CONFIG_OMAP_USB2)+= omap-usb2.o
obj-$(CONFIG_OMAP_USB3)+= omap-usb3.o
obj-$(CONFIG_OMAP_CONTROL_USB) += omap-control-usb.o
diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c
new file mode 100644
index 000..e6f3b74
--- /dev/null
+++ b/drivers/usb/phy/of.c
@@ -0,0 +1,47 @@
+/*
+ * USB of helper code
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+static const char *usbphy_modes[] = {
+ [USBPHY_INTERFACE_MODE_UNKNOWN] = "",
+ [USBPHY_INTERFACE_MODE_UTMI]= "utmi",
+ [USBPHY_INTERFACE_MODE_UTMIW] = "utmi_wide",
+ [USBPHY_INTERFACE_MODE_ULPI]= "ulpi",
+ [USBPHY_INTERFACE_MODE_SERIAL] = "serial",
+ [USBPHY_INTERFACE_MODE_HSIC]= "hsic",
+};
+
+/**
+ * of_usb_get_phy_mode - Get phy mode for given device_node
+ * @np:Pointer to the given device_node
+ *
+ * The function gets phy interface string from property 'phy_type',
+ * and returns the correspondig enum usb_phy_interface
+ */
+enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np)
+{
+ const char *phy_type;
+ int err, i;
+
+ err = of_property_read_string(np, "phy_type", &phy_type);
+ if (err < 0)
+ return USBPHY_INTERFACE_MODE_UNKNOWN;
+
+ for (i = 0; i < ARRAY_SIZE(usbphy_modes); i++)
+ if (!strcmp(phy_type, usbphy_modes[i]))
+ return i;
+
+ return USBPHY_INTERFACE_MODE_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_phy_mode);
diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
index d29503e..18b994b 100644
--- a/drivers/usb/usb-common.c
+++ b/drivers/usb/usb-common.c
@@ -14,6 +14,9 @@
#include
#include
#include
+#include
+#include
+#include
const char *usb_speed_string(enum usb_device_speed speed)
{
@@ -32,4 +35,38 @@ const char *usb_speed_string(enum usb_device_speed speed)
}
EXPORT_SYMBOL_GPL(usb_speed_string);
+#ifdef CONFIG_OF
+static const char *usb_dr_modes[] = {
+ [USB_DR_MODE_UNKNOWN] = "",
+ [USB_DR_MODE_HOST] = "host",
+ [USB_DR_MODE_PERIPHERAL]= "peripheral",
+ [USB_DR_MODE_OTG] = "otg",
+ [USB_DR_MODE_DUAL_ROLE] = "dual-role",
+};
+
+/**
+ * of_usb_get_dr_mode - Get dual role mode for given device_node
+ * @np:Pointer to the given device_node
+ *
+ * The function gets phy interface string from property 'dr_mode',
+ * and returns the correspondig enum usb_dr_mode
+ */
+enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
+{
+ const char *dr_mode;
+ int err, i;
+
+ err = of_property_read_string(np, "dr_mode", &dr_mode);
+ if (err < 0)
+ return USB_DR_MODE_UNKNOWN;
+
+ for (i = 0; i < ARRAY_SIZE(usb_dr_modes); i++)
+ if (!strcmp(dr_mode, usb_dr_modes[i]))
+ return i;
+
+ return USB_DR_MODE_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_dr_mode);
+#endif
+
MODULE_LICENSE("GPL");
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
new file mode 100644
index 000..4681a20
--- /dev/null
+++ b/include/linux/usb/of.h
@@ -0,0 +1,27 @@
+/*
+ * OF helpers for usb devices.
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_USB_OF_H
+#define __LINUX_USB_OF_H
+
+#include
+
+#ifdef CONFIG_OF
+enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np);
+enum usb_dr_mode of_usb_get_dr_mode(str
[PATCH 1/3] USB: move bulk of otg/otg.c to phy/phy.c
From: Sascha Hauer
Most of otg/otg.c is not otg specific, but phy specific, so move it
to the phy directory.
Cc: Felipe Balbi
Reported-by: Kishon Vijay Abraham I
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/otg/otg.c| 427
drivers/usb/phy/Makefile |1 +
drivers/usb/phy/phy.c| 438 ++
3 files changed, 439 insertions(+), 427 deletions(-)
create mode 100644 drivers/usb/phy/phy.c
diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
index 2bd03d2..358cfd9 100644
--- a/drivers/usb/otg/otg.c
+++ b/drivers/usb/otg/otg.c
@@ -8,436 +8,9 @@
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*/
-
-#include
#include
-#include
-#include
-#include
-#include
-#include
-
#include
-static LIST_HEAD(phy_list);
-static LIST_HEAD(phy_bind_list);
-static DEFINE_SPINLOCK(phy_lock);
-
-static struct usb_phy *__usb_find_phy(struct list_head *list,
- enum usb_phy_type type)
-{
- struct usb_phy *phy = NULL;
-
- list_for_each_entry(phy, list, head) {
- if (phy->type != type)
- continue;
-
- return phy;
- }
-
- return ERR_PTR(-ENODEV);
-}
-
-static struct usb_phy *__usb_find_phy_dev(struct device *dev,
- struct list_head *list, u8 index)
-{
- struct usb_phy_bind *phy_bind = NULL;
-
- list_for_each_entry(phy_bind, list, list) {
- if (!(strcmp(phy_bind->dev_name, dev_name(dev))) &&
- phy_bind->index == index) {
- if (phy_bind->phy)
- return phy_bind->phy;
- else
- return ERR_PTR(-EPROBE_DEFER);
- }
- }
-
- return ERR_PTR(-ENODEV);
-}
-
-static struct usb_phy *__of_usb_find_phy(struct device_node *node)
-{
- struct usb_phy *phy;
-
- list_for_each_entry(phy, &phy_list, head) {
- if (node != phy->dev->of_node)
- continue;
-
- return phy;
- }
-
- return ERR_PTR(-ENODEV);
-}
-
-static void devm_usb_phy_release(struct device *dev, void *res)
-{
- struct usb_phy *phy = *(struct usb_phy **)res;
-
- usb_put_phy(phy);
-}
-
-static int devm_usb_phy_match(struct device *dev, void *res, void *match_data)
-{
- return res == match_data;
-}
-
-/**
- * devm_usb_get_phy - find the USB PHY
- * @dev - device that requests this phy
- * @type - the type of the phy the controller requires
- *
- * Gets the phy using usb_get_phy(), and associates a device with it using
- * devres. On driver detach, release function is invoked on the devres data,
- * then, devres data is freed.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *devm_usb_get_phy(struct device *dev, enum usb_phy_type type)
-{
- struct usb_phy **ptr, *phy;
-
- ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
- if (!ptr)
- return NULL;
-
- phy = usb_get_phy(type);
- if (!IS_ERR(phy)) {
- *ptr = phy;
- devres_add(dev, ptr);
- } else
- devres_free(ptr);
-
- return phy;
-}
-EXPORT_SYMBOL(devm_usb_get_phy);
-
-/**
- * usb_get_phy - find the USB PHY
- * @type - the type of the phy the controller requires
- *
- * Returns the phy driver, after getting a refcount to it; or
- * -ENODEV if there is no such phy. The caller is responsible for
- * calling usb_put_phy() to release that count.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *usb_get_phy(enum usb_phy_type type)
-{
- struct usb_phy *phy = NULL;
- unsigned long flags;
-
- spin_lock_irqsave(&phy_lock, flags);
-
- phy = __usb_find_phy(&phy_list, type);
- if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
- pr_err("unable to find transceiver of type %s\n",
- usb_phy_type_string(type));
- goto err0;
- }
-
- get_device(phy->dev);
-
-err0:
- spin_unlock_irqrestore(&phy_lock, flags);
-
- return phy;
-}
-EXPORT_SYMBOL(usb_get_phy);
-
- /**
- * devm_usb_get_phy_by_phandle - find the USB PHY by phandle
- * @dev - device that requests this phy
- * @phandle - name of the property holding the phy phandle value
- * @index - the index of the phy
- *
- * Returns the phy driver associated with the given phandle value,
- * after getting a refcount to it, -ENODEV if there is no such phy or
- * -EPROBE_DEFER if there is a phandle to the phy, but the device is
- * not yet loaded. While at that, it also associates the device with
- * the phy using devres. On driver detach, release function is invoked
- * on the devres data, then, devre
[PATCH 3/3] USB mxs-phy: Register phy with framework
From: Sascha Hauer
We now have usb_add_phy_dev(), so use it to register with the framework
to be able to find the phy from the USB driver.
Reviewed-by: Kishon Vijay Abraham I
Reviewed-by: Peter Chen
Acked-by: Felipe Balbi
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/otg/mxs-phy.c |9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
index b0d9f11..aa40325 100644
--- a/drivers/usb/otg/mxs-phy.c
+++ b/drivers/usb/otg/mxs-phy.c
@@ -127,6 +127,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
void __iomem *base;
struct clk *clk;
struct mxs_phy *mxs_phy;
+ int ret;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -166,11 +167,19 @@ static int mxs_phy_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, &mxs_phy->phy);
+ ret = usb_add_phy_dev(&mxs_phy->phy);
+ if (ret)
+ return ret;
+
return 0;
}
static int mxs_phy_remove(struct platform_device *pdev)
{
+ struct mxs_phy *mxs_phy = platform_get_drvdata(pdev);
+
+ usb_remove_phy(&mxs_phy->phy);
+
platform_set_drvdata(pdev, NULL);
return 0;
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] USB chipidea: add PTW and PTS handling
From: Michael Grzeschik
This patch makes it possible to configure the PTW and PTS bits inside
the portsc register for host and device mode before the driver starts
and the phy can be addressed as hardware implementation is designed.
Signed-off-by: Michael Grzeschik
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
.../devicetree/bindings/usb/ci13xxx-imx.txt|5 +++
drivers/usb/chipidea/bits.h| 14 ++-
drivers/usb/chipidea/ci13xxx_imx.c |3 ++
drivers/usb/chipidea/core.c| 39
include/linux/usb/chipidea.h |1 +
5 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
index 5778b9c..dd42ccd 100644
--- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
+++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
@@ -5,6 +5,11 @@ Required properties:
- reg: Should contain registers location and length
- interrupts: Should contain controller interrupt
+Recommended properies:
+- phy_type: the type of the phy connected to the core. Should be one
+ of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this
+ property the PORTSC register won't be touched
+
Optional properties:
- fsl,usbphy: phandler of usb phy that connects to the only one port
- fsl,usbmisc: phandler of non-core register device, with one argument
diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
index 050de85..d8ffc2f 100644
--- a/drivers/usb/chipidea/bits.h
+++ b/drivers/usb/chipidea/bits.h
@@ -48,10 +48,22 @@
#define PORTSC_SUSP BIT(7)
#define PORTSC_HSPBIT(9)
#define PORTSC_PTC(0x0FUL << 16)
+/* PTS and PTW for non lpm version only */
+#define PORTSC_PTS(d) d) & 0x3) << 30) | (((d) & 0x4) ? BIT(25) :
0))
+#define PORTSC_PTWBIT(28)
/* DEVLC */
#define DEVLC_PSPD(0x03UL << 25)
-#defineDEVLC_PSPD_HS (0x02UL << 25)
+#define DEVLC_PSPD_HS (0x02UL << 25)
+#define DEVLC_PTW BIT(27)
+#define DEVLC_STS BIT(28)
+#define DEVLC_PTS(d) (((d) & 0x7) << 29)
+
+/* Encoding for DEVLC_PTS and PORTSC_PTS */
+#define PTS_UTMI 0
+#define PTS_ULPI 2
+#define PTS_SERIAL3
+#define PTS_HSIC 4
/* OTGSC */
#define OTGSC_IDPU BIT(5)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index 69024e0..ebc1148 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include "ci.h"
#include "ci13xxx_imx.h"
@@ -112,6 +113,8 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
CI13XXX_PULLUP_ON_VBUS |
CI13XXX_DISABLE_STREAMING;
+ pdata->phy_mode = of_usb_get_phy_mode(pdev->dev.of_node);
+
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
if (!data) {
dev_err(&pdev->dev, "Failed to allocate CI13xxx-IMX data!\n");
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 57cae1f..04d68cb 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -67,6 +67,8 @@
#include
#include
#include
+#include
+#include
#include "ci.h"
#include "udc.h"
@@ -211,6 +213,41 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem
*base)
return 0;
}
+static void hw_phymode_configure(struct ci13xxx *ci)
+{
+ u32 portsc, lpm;
+
+ switch (ci->platdata->phy_mode) {
+ case USBPHY_INTERFACE_MODE_UTMI:
+ portsc = PORTSC_PTS(PTS_UTMI);
+ lpm = DEVLC_PTS(PTS_UTMI);
+ break;
+ case USBPHY_INTERFACE_MODE_UTMIW:
+ portsc = PORTSC_PTS(PTS_UTMI) | PORTSC_PTW;
+ lpm = DEVLC_PTS(PTS_UTMI) | DEVLC_PTW;
+ break;
+ case USBPHY_INTERFACE_MODE_ULPI:
+ portsc = PORTSC_PTS(PTS_ULPI);
+ lpm = DEVLC_PTS(PTS_ULPI);
+ break;
+ case USBPHY_INTERFACE_MODE_SERIAL:
+ portsc = PORTSC_PTS(PTS_SERIAL);
+ lpm = DEVLC_PTS(PTS_SERIAL);
+ break;
+ case USBPHY_INTERFACE_MODE_HSIC:
+ portsc = PORTSC_PTS(PTS_HSIC);
+ lpm = DEVLC_PTS(PTS_HSIC);
+ break;
+ default:
+ return;
+ }
+
+ if (ci->hw_bank.lpm)
+ hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
+ else
+ hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW
[PATCH 4/5] USB chipidea i.MX: introduce dr_mode property
From: Sascha Hauer
The dr_mode devicetree property allows to explicitly specify the
host/peripheral/otg mode. This is necessary for boards without proper
ID pin handling.
Reviewed-by: Peter Chen
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
Documentation/devicetree/bindings/usb/ci13xxx-imx.txt |1 +
drivers/usb/chipidea/ci13xxx_imx.c|1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
index dd42ccd..493a414 100644
--- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
+++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
@@ -9,6 +9,7 @@ Recommended properies:
- phy_type: the type of the phy connected to the core. Should be one
of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this
property the PORTSC register won't be touched
+- dr_mode: One of "host", "peripheral" or "otg". Defaults to "otg"
Optional properties:
- fsl,usbphy: phandler of usb phy that connects to the only one port
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index ebc1148..b598bb8 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -114,6 +114,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
CI13XXX_DISABLE_STREAMING;
pdata->phy_mode = of_usb_get_phy_mode(pdev->dev.of_node);
+ pdata->dr_mode = of_usb_get_dr_mode(pdev->dev.of_node);
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
if (!data) {
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] chipidea-for-v3.10-v1: USB chipidea: make use of DT helpers in chipidea driver improve driver
Hello, this series depends on the series "[PATCH 0/3] otg-for-v3.10-v1: separate phy code and add DT helper" (a.k.a. tags/otg-for-v3.10-v1) posted earlier and is intended for v3.10. The chipidea driver is converted to make use of the DT helper functions. regards, Marc The following changes since commit 8a4c9f8489b2fce9e7bf0eb43fdb160ab51adc2c: USB mxs-phy: Register phy with framework (2013-02-27 13:48:21 +0100) are available in the git repository at: git://git.pengutronix.de/git/mkl/linux.git tags/chipidea-for-v3.10-v1 for you to fetch changes up to 5d837224ea36da50f3e744a65fb23af1fa0a480d: USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy (2013-02-27 13:49:07 +0100) USB chipidea: make use of DT helpers in chipidea driver Make use of DT helper functions for handling the dr_mode and phy_type property. Michael Grzeschik (2): USB chipidea: ci13xxx-imx: create dynamic platformdata USB chipidea: add PTW and PTS handling Sascha Hauer (3): USB chipidea: introduce dual role mode pdata flags USB chipidea i.MX: introduce dr_mode property USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy .../devicetree/bindings/usb/ci13xxx-imx.txt|6 ++ drivers/usb/chipidea/bits.h| 14 +++- drivers/usb/chipidea/ci13xxx_imx.c | 67 +++- drivers/usb/chipidea/core.c| 61 -- include/linux/usb/chipidea.h |3 +- 5 files changed, 112 insertions(+), 39 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] USB chipidea: ci13xxx-imx: create dynamic platformdata
From: Michael Grzeschik
This patch removes the limitation of having only one instance of the
ci13xxx-imx platformdata and makes different configurations possible.
Reviewed-by: Peter Chen
Signed-off-by: Michael Grzeschik
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/chipidea/ci13xxx_imx.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index 8c29122..69024e0 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -85,17 +85,10 @@ EXPORT_SYMBOL_GPL(usbmisc_get_init_data);
/* End of common functions shared by usbmisc drivers*/
-static struct ci13xxx_platform_data ci13xxx_imx_platdata = {
- .name = "ci13xxx_imx",
- .flags = CI13XXX_REQUIRE_TRANSCEIVER |
- CI13XXX_PULLUP_ON_VBUS |
- CI13XXX_DISABLE_STREAMING,
- .capoffset = DEF_CAPOFFSET,
-};
-
static int ci13xxx_imx_probe(struct platform_device *pdev)
{
struct ci13xxx_imx_data *data;
+ struct ci13xxx_platform_data *pdata;
struct platform_device *plat_ci, *phy_pdev;
struct device_node *phy_np;
struct resource *res;
@@ -107,6 +100,18 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
&& !usbmisc_ops)
return -EPROBE_DEFER;
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ dev_err(&pdev->dev, "Failed to allocate CI13xxx-IMX pdata!\n");
+ return -ENOMEM;
+ }
+
+ pdata->name = "ci13xxx_imx";
+ pdata->capoffset = DEF_CAPOFFSET;
+ pdata->flags = CI13XXX_REQUIRE_TRANSCEIVER |
+ CI13XXX_PULLUP_ON_VBUS |
+ CI13XXX_DISABLE_STREAMING;
+
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
if (!data) {
dev_err(&pdev->dev, "Failed to allocate CI13xxx-IMX data!\n");
@@ -168,7 +173,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
reg_vbus = NULL;
}
- ci13xxx_imx_platdata.phy = data->phy;
+ pdata->phy = data->phy;
if (!pdev->dev.dma_mask) {
pdev->dev.dma_mask = devm_kzalloc(&pdev->dev,
@@ -193,7 +198,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
plat_ci = ci13xxx_add_device(&pdev->dev,
pdev->resource, pdev->num_resources,
- &ci13xxx_imx_platdata);
+ pdata);
if (IS_ERR(plat_ci)) {
ret = PTR_ERR(plat_ci);
dev_err(&pdev->dev,
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy
From: Sascha Hauer
This patch replaces the hand crafted code to retrieve the phy's phandle from
the DT by the helper function devm_usb_get_phy_by_phandle() which has been
added in commit:
"5d3c28b usb: otg: add device tree support to otg library"
Reviewed-by: Kishon Vijay Abraham I
Reviewed-by: Peter Chen
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/chipidea/ci13xxx_imx.c | 38
1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index b598bb8..6e720ce 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -30,7 +30,6 @@
((struct usb_phy *)platform_get_drvdata(pdev))
struct ci13xxx_imx_data {
- struct device_node *phy_np;
struct usb_phy *phy;
struct platform_device *ci_pdev;
struct clk *clk;
@@ -90,12 +89,12 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
{
struct ci13xxx_imx_data *data;
struct ci13xxx_platform_data *pdata;
- struct platform_device *plat_ci, *phy_pdev;
- struct device_node *phy_np;
+ struct platform_device *plat_ci;
struct resource *res;
struct regulator *reg_vbus;
struct pinctrl *pinctrl;
int ret;
+ struct usb_phy *phy;
if (of_find_property(pdev->dev.of_node, "fsl,usbmisc", NULL)
&& !usbmisc_ops)
@@ -147,19 +146,20 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
return ret;
}
- phy_np = of_parse_phandle(pdev->dev.of_node, "fsl,usbphy", 0);
- if (phy_np) {
- data->phy_np = phy_np;
- phy_pdev = of_find_device_by_node(phy_np);
- if (phy_pdev) {
- struct usb_phy *phy;
- phy = pdev_to_phy(phy_pdev);
- if (phy &&
- try_module_get(phy_pdev->dev.driver->owner)) {
- usb_phy_init(phy);
- data->phy = phy;
- }
+ phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0);
+ if (PTR_ERR(phy) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto err_clk;
+ }
+
+ if (!IS_ERR(phy)) {
+ ret = usb_phy_init(phy);
+ if (ret) {
+ dev_err(&pdev->dev, "unable to init phy: %d\n", ret);
+ goto err_clk;
}
+
+ data->phy = phy;
}
/* we only support host now, so enable vbus here */
@@ -170,7 +170,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
dev_err(&pdev->dev,
"Failed to enable vbus regulator, err=%d\n",
ret);
- goto put_np;
+ goto err_clk;
}
data->reg_vbus = reg_vbus;
} else {
@@ -222,9 +222,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
err:
if (reg_vbus)
regulator_disable(reg_vbus);
-put_np:
- if (phy_np)
- of_node_put(phy_np);
+err_clk:
clk_disable_unprepare(data->clk);
return ret;
}
@@ -244,8 +242,6 @@ static int ci13xxx_imx_remove(struct platform_device *pdev)
module_put(data->phy->dev->driver->owner);
}
- of_node_put(data->phy_np);
-
clk_disable_unprepare(data->clk);
platform_set_drvdata(pdev, NULL);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] USB chipidea: introduce dual role mode pdata flags
From: Sascha Hauer
Even if a chipidea core is otg capable the board may not. This allows
to explicitly set the core to host/peripheral mode. Without these
flags the driver falls back to the old behaviour.
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/chipidea/core.c | 22 --
include/linux/usb/chipidea.h |2 +-
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 04d68cb..ec27060 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -435,6 +435,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
struct resource *res;
void __iomem*base;
int ret;
+ enum usb_dr_mode dr_mode;
if (!dev->platform_data) {
dev_err(dev, "platform data missing\n");
@@ -487,14 +488,23 @@ static int ci_hdrc_probe(struct platform_device *pdev)
return -ENODEV;
}
+ /* For now we treat dual-role as otg */
+ dr_mode = ci->platdata->dr_mode;
+ if (dr_mode == USB_DR_MODE_UNKNOWN || dr_mode == USB_DR_MODE_DUAL_ROLE)
+ dr_mode = USB_DR_MODE_OTG;
+
/* initialize role(s) before the interrupt is requested */
- ret = ci_hdrc_host_init(ci);
- if (ret)
- dev_info(dev, "doesn't support host\n");
+ if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
+ ret = ci_hdrc_host_init(ci);
+ if (ret)
+ dev_info(dev, "doesn't support host\n");
+ }
- ret = ci_hdrc_gadget_init(ci);
- if (ret)
- dev_info(dev, "doesn't support gadget\n");
+ if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
+ ret = ci_hdrc_gadget_init(ci);
+ if (ret)
+ dev_info(dev, "doesn't support gadget\n");
+ }
if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
dev_err(dev, "no supported roles\n");
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 1a2aa18..b314647 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -20,7 +20,7 @@ struct ci13xxx_platform_data {
#define CI13XXX_REQUIRE_TRANSCEIVERBIT(1)
#define CI13XXX_PULLUP_ON_VBUS BIT(2)
#define CI13XXX_DISABLE_STREAMING BIT(3)
-
+ enum usb_dr_modedr_mode;
#define CI13XXX_CONTROLLER_RESET_EVENT 0
#define CI13XXX_CONTROLLER_STOPPED_EVENT 1
void(*notify_event) (struct ci13xxx *ci, unsigned event);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] USB otg: use try_module_get in all usb_get_phy functions and add missing module_put
On 02/27/2013 02:55 PM, Marc Kleine-Budde wrote: > On 02/27/2013 02:42 PM, Felipe Balbi wrote: >>> That'll be these 3 branches then: >>> >>> otg-for-v3.9 (bugfix for v3.9): >>> 6bef020 USB otg: use try_module_get in all usb_get_phy functions and add >>> missing module_put >>> >>> otg-for-v3.10 (depends on otg-for-v3.9): >>> a0e17f5 USB: move bulk of otg/otg.c to phy/phy.c >>> a6fc0d1 USB: add devicetree helpers for determining dr_mode and phy_type >>> 8a4c9f8 USB mxs-phy: Register phy with framework >>> >>> chipidea-for-v3.10 (most patches depend on otg-for-v3.10): >>> 15c930e USB chipidea: ci13xxx-imx: create dynamic platformdata >>> 9466e85 USB chipidea: add PTW and PTS handling >>> ad2cc0d USB chipidea: introduce dual role mode pdata flags >>> 8e7f1bb USB chipidea i.MX: introduce dr_mode property >>> 5d83722 USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy >> >> sounds good to me, please make sure to always split your series like >> this. You can't expect me (or any maintainer for that matter) to pick >> patches to proper branches based on your commit logs alone. >> >> remeber that every single maintainer needs to handle hundreds of emails >> a week (if not a day) and if patches are properly split and cleanly >> mention if it's a bugfix or not, things will go wrong. > > That was just a proposal. You'll find these series in your inbox in a > few minutes. Done. regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH] USB otg: use try_module_get in all usb_get_phy functions and add missing module_put
On 02/27/2013 05:21 PM, Greg KH wrote:
> On Wed, Feb 27, 2013 at 03:11:13PM +0100, Marc Kleine-Budde wrote:
>> In patch "5d3c28b usb: otg: add device tree support to otg library"
>> devm_usb_get_phy_by_phandle() was added. It uses try_module_get() to lock the
>> phy driver in memory. The corresponding module_put() is missing in that
>> patch.
>>
>> This patch adds try_module_get() to usb_get_phy() and usb_get_phy_dev().
>> Further the missing module_put() is added to usb_put_phy().
>>
>> Reviewed-by: Kishon Vijay Abraham I
>> Acked-by: Felipe Balbi
>> Signed-off-by: Marc Kleine-Budde
>> Signed-off-by: Michael Grzeschik
>> ---
>> drivers/usb/otg/otg.c | 10 +++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
>> index e181439..2bd03d2 100644
>> --- a/drivers/usb/otg/otg.c
>> +++ b/drivers/usb/otg/otg.c
>> @@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
>> spin_lock_irqsave(&phy_lock, flags);
>>
>> phy = __usb_find_phy(&phy_list, type);
>> -if (IS_ERR(phy)) {
>> +if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
>
> Ugh, really? We really are trying this type of module locking mess?
>
> Why? What is it solving? What's wrong with having the module be able
Without this patch, you can unload the phy module, while it is in use.
As the phy framework doesn't use any existing abstraction to handle phy
<-> user of phy pairing/unpairing (it's all open coded), any subsequent
use of the phy's callback will result in deref'ing bogus pointers.
> to be unloaded whenever it wants to? No one should be doing that and
> expecting that their hardware would still work properly, right?
Yes, but it should not result in an kernel oops.
> I really don't like this type of thing, sorry.
Can you point Kishon and me to a better implementation?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH 3/3] USB mxs-phy: Register phy with framework
On 02/28/2013 09:01 AM, Felipe Balbi wrote:
> hi,
>
> On Wed, Feb 27, 2013 at 03:16:30PM +0100, Marc Kleine-Budde wrote:
>> From: Sascha Hauer
>>
>> We now have usb_add_phy_dev(), so use it to register with the framework
>> to be able to find the phy from the USB driver.
>>
>> Reviewed-by: Kishon Vijay Abraham I
>> Reviewed-by: Peter Chen
>> Acked-by: Felipe Balbi
>> Signed-off-by: Sascha Hauer
>> Signed-off-by: Marc Kleine-Budde
>
> any chance you can move away from {write,read}[bwl]_relaxed() so we can
> build this driver on other architectures ?
The hardware is in the ARM imx2{2,8} only. Another option would be to
add an "depends on ARCH_ARM".
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH 2/3] USB: add devicetree helpers for determining dr_mode and phy_type
On 02/28/2013 08:57 AM, Felipe Balbi wrote: > On Wed, Feb 27, 2013 at 03:16:29PM +0100, Marc Kleine-Budde wrote: >> From: Michael Grzeschik >> >> This adds two little devicetree helper functions for determining the dr_mode >> (host, peripheral, otg, dual-role) and phy_type (utmi, ulpi,...) from the >> devicetree. >> >> Cc: Felipe Balbi >> Signed-off-by: Michael Grzeschik >> Signed-off-by: Sascha Hauer >> Signed-off-by: Marc Kleine-Budde > > causes build breakage: > > include/linux/usb/of.h:21:32: error: return type is an incomplete type > include/linux/usb/of.h: In function ‘of_usb_get_dr_mode’: > include/linux/usb/of.h:23:9: error: ‘USB_DR_MODE_UNKNOWN’ undeclared (first > use in this function) > include/linux/usb/of.h:23:9: note: each undeclared identifier is reported > only once for each function it appears in > include/linux/usb/of.h:23:2: warning: ‘return’ with a value, in function > returning void [enabled by default] > make[1]: *** [drivers/usb/usb-common.o] Error 1 Doh! That occurs only for non DT kernels, but who doesn't use device tree these days? Fixed, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
[PATCH 2/4] USB: add devicetree helpers for determining dr_mode and phy_type
From: Michael Grzeschik
This adds two little devicetree helper functions for determining the dr_mode
(host, peripheral, otg, dual-role) and phy_type (utmi, ulpi,...) from the
devicetree.
Cc: Felipe Balbi
Signed-off-by: Michael Grzeschik
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/phy/Makefile |1 +
drivers/usb/phy/of.c | 47 ++
drivers/usb/usb-common.c | 37
include/linux/usb/of.h | 28 +++
include/linux/usb/otg.h |8
include/linux/usb/phy.h |9 +
6 files changed, 130 insertions(+)
create mode 100644 drivers/usb/phy/of.c
create mode 100644 include/linux/usb/of.h
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 9fa6327..e1be1e8 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -5,6 +5,7 @@
ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
obj-$(CONFIG_USB_OTG_UTILS)+= phy.o
+obj-$(CONFIG_OF) += of.o
obj-$(CONFIG_OMAP_USB2)+= omap-usb2.o
obj-$(CONFIG_OMAP_USB3)+= omap-usb3.o
obj-$(CONFIG_OMAP_CONTROL_USB) += omap-control-usb.o
diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c
new file mode 100644
index 000..e6f3b74
--- /dev/null
+++ b/drivers/usb/phy/of.c
@@ -0,0 +1,47 @@
+/*
+ * USB of helper code
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+static const char *usbphy_modes[] = {
+ [USBPHY_INTERFACE_MODE_UNKNOWN] = "",
+ [USBPHY_INTERFACE_MODE_UTMI]= "utmi",
+ [USBPHY_INTERFACE_MODE_UTMIW] = "utmi_wide",
+ [USBPHY_INTERFACE_MODE_ULPI]= "ulpi",
+ [USBPHY_INTERFACE_MODE_SERIAL] = "serial",
+ [USBPHY_INTERFACE_MODE_HSIC]= "hsic",
+};
+
+/**
+ * of_usb_get_phy_mode - Get phy mode for given device_node
+ * @np:Pointer to the given device_node
+ *
+ * The function gets phy interface string from property 'phy_type',
+ * and returns the correspondig enum usb_phy_interface
+ */
+enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np)
+{
+ const char *phy_type;
+ int err, i;
+
+ err = of_property_read_string(np, "phy_type", &phy_type);
+ if (err < 0)
+ return USBPHY_INTERFACE_MODE_UNKNOWN;
+
+ for (i = 0; i < ARRAY_SIZE(usbphy_modes); i++)
+ if (!strcmp(phy_type, usbphy_modes[i]))
+ return i;
+
+ return USBPHY_INTERFACE_MODE_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_phy_mode);
diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
index d29503e..18b994b 100644
--- a/drivers/usb/usb-common.c
+++ b/drivers/usb/usb-common.c
@@ -14,6 +14,9 @@
#include
#include
#include
+#include
+#include
+#include
const char *usb_speed_string(enum usb_device_speed speed)
{
@@ -32,4 +35,38 @@ const char *usb_speed_string(enum usb_device_speed speed)
}
EXPORT_SYMBOL_GPL(usb_speed_string);
+#ifdef CONFIG_OF
+static const char *usb_dr_modes[] = {
+ [USB_DR_MODE_UNKNOWN] = "",
+ [USB_DR_MODE_HOST] = "host",
+ [USB_DR_MODE_PERIPHERAL]= "peripheral",
+ [USB_DR_MODE_OTG] = "otg",
+ [USB_DR_MODE_DUAL_ROLE] = "dual-role",
+};
+
+/**
+ * of_usb_get_dr_mode - Get dual role mode for given device_node
+ * @np:Pointer to the given device_node
+ *
+ * The function gets phy interface string from property 'dr_mode',
+ * and returns the correspondig enum usb_dr_mode
+ */
+enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
+{
+ const char *dr_mode;
+ int err, i;
+
+ err = of_property_read_string(np, "dr_mode", &dr_mode);
+ if (err < 0)
+ return USB_DR_MODE_UNKNOWN;
+
+ for (i = 0; i < ARRAY_SIZE(usb_dr_modes); i++)
+ if (!strcmp(dr_mode, usb_dr_modes[i]))
+ return i;
+
+ return USB_DR_MODE_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_dr_mode);
+#endif
+
MODULE_LICENSE("GPL");
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
new file mode 100644
index 000..fc98f68
--- /dev/null
+++ b/include/linux/usb/of.h
@@ -0,0 +1,28 @@
+/*
+ * OF helpers for usb devices.
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_USB_OF_H
+#define __LINUX_USB_OF_H
+
+#include
+#include
+
+#ifdef CONFIG_OF
+enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np);
+enum usb_dr_mode o
[PATCH 4/4] USB mxs-phy: Register phy with framework
From: Sascha Hauer
We now have usb_add_phy_dev(), so use it to register with the framework
to be able to find the phy from the USB driver.
Reviewed-by: Kishon Vijay Abraham I
Reviewed-by: Peter Chen
Acked-by: Felipe Balbi
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/otg/mxs-phy.c |9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
index dff10f2..9d4381e 100644
--- a/drivers/usb/otg/mxs-phy.c
+++ b/drivers/usb/otg/mxs-phy.c
@@ -127,6 +127,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
void __iomem *base;
struct clk *clk;
struct mxs_phy *mxs_phy;
+ int ret;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -166,11 +167,19 @@ static int mxs_phy_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, &mxs_phy->phy);
+ ret = usb_add_phy_dev(&mxs_phy->phy);
+ if (ret)
+ return ret;
+
return 0;
}
static int mxs_phy_remove(struct platform_device *pdev)
{
+ struct mxs_phy *mxs_phy = platform_get_drvdata(pdev);
+
+ usb_remove_phy(&mxs_phy->phy);
+
platform_set_drvdata(pdev, NULL);
return 0;
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] USB mxs-phy: use readl(), writel() instead of the _relaxed() versions
This patch converts the mxs-phy driver from readl_relaxed(), writel_relaxed()
to the plain readl(), writel() functions, which are available on all platforms.
This is done to enable compile time testing on non ARM platforms.
Reported-by: Alexander Shishkin
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/otg/mxs-phy.c | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
index b0d9f11..dff10f2 100644
--- a/drivers/usb/otg/mxs-phy.c
+++ b/drivers/usb/otg/mxs-phy.c
@@ -48,12 +48,12 @@ static void mxs_phy_hw_init(struct mxs_phy *mxs_phy)
stmp_reset_block(base + HW_USBPHY_CTRL);
/* Power up the PHY */
- writel_relaxed(0, base + HW_USBPHY_PWD);
+ writel(0, base + HW_USBPHY_PWD);
/* enable FS/LS device */
- writel_relaxed(BM_USBPHY_CTRL_ENUTMILEVEL2 |
- BM_USBPHY_CTRL_ENUTMILEVEL3,
- base + HW_USBPHY_CTRL_SET);
+ writel(BM_USBPHY_CTRL_ENUTMILEVEL2 |
+ BM_USBPHY_CTRL_ENUTMILEVEL3,
+ base + HW_USBPHY_CTRL_SET);
}
static int mxs_phy_init(struct usb_phy *phy)
@@ -70,8 +70,8 @@ static void mxs_phy_shutdown(struct usb_phy *phy)
{
struct mxs_phy *mxs_phy = to_mxs_phy(phy);
- writel_relaxed(BM_USBPHY_CTRL_CLKGATE,
- phy->io_priv + HW_USBPHY_CTRL_SET);
+ writel(BM_USBPHY_CTRL_CLKGATE,
+ phy->io_priv + HW_USBPHY_CTRL_SET);
clk_disable_unprepare(mxs_phy->clk);
}
@@ -81,15 +81,15 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
struct mxs_phy *mxs_phy = to_mxs_phy(x);
if (suspend) {
- writel_relaxed(0x, x->io_priv + HW_USBPHY_PWD);
- writel_relaxed(BM_USBPHY_CTRL_CLKGATE,
- x->io_priv + HW_USBPHY_CTRL_SET);
+ writel(0x, x->io_priv + HW_USBPHY_PWD);
+ writel(BM_USBPHY_CTRL_CLKGATE,
+ x->io_priv + HW_USBPHY_CTRL_SET);
clk_disable_unprepare(mxs_phy->clk);
} else {
clk_prepare_enable(mxs_phy->clk);
- writel_relaxed(BM_USBPHY_CTRL_CLKGATE,
- x->io_priv + HW_USBPHY_CTRL_CLR);
- writel_relaxed(0, x->io_priv + HW_USBPHY_PWD);
+ writel(BM_USBPHY_CTRL_CLKGATE,
+ x->io_priv + HW_USBPHY_CTRL_CLR);
+ writel(0, x->io_priv + HW_USBPHY_PWD);
}
return 0;
@@ -102,8 +102,8 @@ static int mxs_phy_on_connect(struct usb_phy *phy,
(speed == USB_SPEED_HIGH) ? "high" : "non-high");
if (speed == USB_SPEED_HIGH)
- writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
- phy->io_priv + HW_USBPHY_CTRL_SET);
+ writel(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
+ phy->io_priv + HW_USBPHY_CTRL_SET);
return 0;
}
@@ -115,8 +115,8 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy,
(speed == USB_SPEED_HIGH) ? "high" : "non-high");
if (speed == USB_SPEED_HIGH)
- writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
- phy->io_priv + HW_USBPHY_CTRL_CLR);
+ writel(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
+ phy->io_priv + HW_USBPHY_CTRL_CLR);
return 0;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] USB: move bulk of otg/otg.c to phy/phy.c
From: Sascha Hauer
Most of otg/otg.c is not otg specific, but phy specific, so move it
to the phy directory.
Cc: Felipe Balbi
Reported-by: Kishon Vijay Abraham I
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/otg/otg.c| 427
drivers/usb/phy/Makefile |1 +
drivers/usb/phy/phy.c| 438 ++
3 files changed, 439 insertions(+), 427 deletions(-)
create mode 100644 drivers/usb/phy/phy.c
diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
index 2bd03d2..358cfd9 100644
--- a/drivers/usb/otg/otg.c
+++ b/drivers/usb/otg/otg.c
@@ -8,436 +8,9 @@
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*/
-
-#include
#include
-#include
-#include
-#include
-#include
-#include
-
#include
-static LIST_HEAD(phy_list);
-static LIST_HEAD(phy_bind_list);
-static DEFINE_SPINLOCK(phy_lock);
-
-static struct usb_phy *__usb_find_phy(struct list_head *list,
- enum usb_phy_type type)
-{
- struct usb_phy *phy = NULL;
-
- list_for_each_entry(phy, list, head) {
- if (phy->type != type)
- continue;
-
- return phy;
- }
-
- return ERR_PTR(-ENODEV);
-}
-
-static struct usb_phy *__usb_find_phy_dev(struct device *dev,
- struct list_head *list, u8 index)
-{
- struct usb_phy_bind *phy_bind = NULL;
-
- list_for_each_entry(phy_bind, list, list) {
- if (!(strcmp(phy_bind->dev_name, dev_name(dev))) &&
- phy_bind->index == index) {
- if (phy_bind->phy)
- return phy_bind->phy;
- else
- return ERR_PTR(-EPROBE_DEFER);
- }
- }
-
- return ERR_PTR(-ENODEV);
-}
-
-static struct usb_phy *__of_usb_find_phy(struct device_node *node)
-{
- struct usb_phy *phy;
-
- list_for_each_entry(phy, &phy_list, head) {
- if (node != phy->dev->of_node)
- continue;
-
- return phy;
- }
-
- return ERR_PTR(-ENODEV);
-}
-
-static void devm_usb_phy_release(struct device *dev, void *res)
-{
- struct usb_phy *phy = *(struct usb_phy **)res;
-
- usb_put_phy(phy);
-}
-
-static int devm_usb_phy_match(struct device *dev, void *res, void *match_data)
-{
- return res == match_data;
-}
-
-/**
- * devm_usb_get_phy - find the USB PHY
- * @dev - device that requests this phy
- * @type - the type of the phy the controller requires
- *
- * Gets the phy using usb_get_phy(), and associates a device with it using
- * devres. On driver detach, release function is invoked on the devres data,
- * then, devres data is freed.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *devm_usb_get_phy(struct device *dev, enum usb_phy_type type)
-{
- struct usb_phy **ptr, *phy;
-
- ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
- if (!ptr)
- return NULL;
-
- phy = usb_get_phy(type);
- if (!IS_ERR(phy)) {
- *ptr = phy;
- devres_add(dev, ptr);
- } else
- devres_free(ptr);
-
- return phy;
-}
-EXPORT_SYMBOL(devm_usb_get_phy);
-
-/**
- * usb_get_phy - find the USB PHY
- * @type - the type of the phy the controller requires
- *
- * Returns the phy driver, after getting a refcount to it; or
- * -ENODEV if there is no such phy. The caller is responsible for
- * calling usb_put_phy() to release that count.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *usb_get_phy(enum usb_phy_type type)
-{
- struct usb_phy *phy = NULL;
- unsigned long flags;
-
- spin_lock_irqsave(&phy_lock, flags);
-
- phy = __usb_find_phy(&phy_list, type);
- if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
- pr_err("unable to find transceiver of type %s\n",
- usb_phy_type_string(type));
- goto err0;
- }
-
- get_device(phy->dev);
-
-err0:
- spin_unlock_irqrestore(&phy_lock, flags);
-
- return phy;
-}
-EXPORT_SYMBOL(usb_get_phy);
-
- /**
- * devm_usb_get_phy_by_phandle - find the USB PHY by phandle
- * @dev - device that requests this phy
- * @phandle - name of the property holding the phy phandle value
- * @index - the index of the phy
- *
- * Returns the phy driver associated with the given phandle value,
- * after getting a refcount to it, -ENODEV if there is no such phy or
- * -EPROBE_DEFER if there is a phandle to the phy, but the device is
- * not yet loaded. While at that, it also associates the device with
- * the phy using devres. On driver detach, release function is invoked
- * on the devres data, then, devre
[PATCH 0/4] otg-for-v3.10-v2: separate phy code and add DT helper
Hello,
this series depends on the bugfix patch "USB otg: use try_module_get in all
usb_get_phy functions and add missing module_put" (a.k.a tags/otg-for-v3.9-v1)
posted earlier and is inteded for v3.10. It separates the phy from the otg code
and adds DT helper functions. In mxs-phy the {read,write}l_relaxed() functions
are replaced by the non _relaxed version to improve compile time coverage.
Further mxs-phy makes now use of the new usb_add_phy_dev() function to register
it's phy.
regards,
Marc
---
changes since v1:
- fix compile time breakage on non DT platforms (tnx, Alexander)
- convert mxs-phy to non _relaxed {read,write}l_relaxed() functions
(as requested by Alexander)
---
The following changes since commit 6bef020b4aebd7886281fb7fb37c788d5a365eea:
USB otg: use try_module_get in all usb_get_phy functions and add missing
module_put (2013-02-27 12:53:15 +0100)
are available in the git repository at:
git://git.pengutronix.de/git/mkl/linux.git tags/otg-for-v3.10-v2
for you to fetch changes up to f5678b135967ea98256ee5df9a360b5769861d23:
USB mxs-phy: Register phy with framework (2013-02-28 11:36:45 +0100)
USB otg, phy: separate phy and add DT helper
Move phy related code into separate file and add device tree helper functions.
--------
Marc Kleine-Budde (1):
USB mxs-phy: use readl(), writel() instead of the _relaxed() versions
Michael Grzeschik (1):
USB: add devicetree helpers for determining dr_mode and phy_type
Sascha Hauer (2):
USB: move bulk of otg/otg.c to phy/phy.c
USB mxs-phy: Register phy with framework
drivers/usb/otg/mxs-phy.c | 41 +++--
drivers/usb/otg/otg.c | 427 ---
drivers/usb/phy/Makefile |2 +
drivers/usb/phy/of.c | 47 +
drivers/usb/phy/phy.c | 438 +
drivers/usb/usb-common.c | 37
include/linux/usb/of.h| 28 +++
include/linux/usb/otg.h |8 +
include/linux/usb/phy.h |9 +
9 files changed, 594 insertions(+), 443 deletions(-)
create mode 100644 drivers/usb/phy/of.c
create mode 100644 drivers/usb/phy/phy.c
create mode 100644 include/linux/usb/of.h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] USB mxs-phy: Register phy with framework
On 02/28/2013 10:40 AM, Alexander Shishkin wrote:
> Marc Kleine-Budde writes:
>
>> On 02/28/2013 09:01 AM, Felipe Balbi wrote:
>>> hi,
>>>
>>> On Wed, Feb 27, 2013 at 03:16:30PM +0100, Marc Kleine-Budde wrote:
>>>> From: Sascha Hauer
>>>>
>>>> We now have usb_add_phy_dev(), so use it to register with the framework
>>>> to be able to find the phy from the USB driver.
>>>>
>>>> Reviewed-by: Kishon Vijay Abraham I
>>>> Reviewed-by: Peter Chen
>>>> Acked-by: Felipe Balbi
>>>> Signed-off-by: Sascha Hauer
>>>> Signed-off-by: Marc Kleine-Budde
>>>
>>> any chance you can move away from {write,read}[bwl]_relaxed() so we can
>>> build this driver on other architectures ?
>>
>> The hardware is in the ARM imx2{2,8} only. Another option would be to
>> add an "depends on ARCH_ARM".
>
> Doesn't mean that we shouldn't be able to compile-test the driver.
Fixed in the just posted otg-for-v3.10-v2 series.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
[PATCH 0/5] chipidea-for-v3.10-v2: USB chipidea: make use of DT helpers in chipidea driver improve driver
Hello, this series depends on the series "[PATCH 0/3] otg-for-v3.10-v2: separate phy code and add DT helper" (a.k.a. tags/otg-for-v3.10-v2) posted earlier and is intended for v3.10. The chipidea driver is converted to make use of the DT helper functions. No changes since v1, just rebased to otg-for-v3.10-v2, in case someone wants to pull it. regards, Marc --- The following changes since commit f5678b135967ea98256ee5df9a360b5769861d23: USB mxs-phy: Register phy with framework (2013-02-28 11:36:45 +0100) are available in the git repository at: git://git.pengutronix.de/git/mkl/linux.git tags/chipidea-for-v3.10-v2 for you to fetch changes up to 11e56207b94d65f92acdee17c795753378c581c6: USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy (2013-02-28 11:36:48 +0100) USB chipidea: make use of DT helpers in chipidea driver Make use of DT helper functions for handling the dr_mode and phy_type property. Michael Grzeschik (2): USB chipidea: ci13xxx-imx: create dynamic platformdata USB chipidea: add PTW and PTS handling Sascha Hauer (3): USB chipidea: introduce dual role mode pdata flags USB chipidea i.MX: introduce dr_mode property USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy .../devicetree/bindings/usb/ci13xxx-imx.txt|6 ++ drivers/usb/chipidea/bits.h| 14 +++- drivers/usb/chipidea/ci13xxx_imx.c | 67 +++- drivers/usb/chipidea/core.c| 61 -- include/linux/usb/chipidea.h |3 +- 5 files changed, 112 insertions(+), 39 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy
From: Sascha Hauer
This patch replaces the hand crafted code to retrieve the phy's phandle from
the DT by the helper function devm_usb_get_phy_by_phandle() which has been
added in commit:
"5d3c28b usb: otg: add device tree support to otg library"
Reviewed-by: Kishon Vijay Abraham I
Reviewed-by: Peter Chen
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/chipidea/ci13xxx_imx.c | 38
1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index b598bb8..6e720ce 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -30,7 +30,6 @@
((struct usb_phy *)platform_get_drvdata(pdev))
struct ci13xxx_imx_data {
- struct device_node *phy_np;
struct usb_phy *phy;
struct platform_device *ci_pdev;
struct clk *clk;
@@ -90,12 +89,12 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
{
struct ci13xxx_imx_data *data;
struct ci13xxx_platform_data *pdata;
- struct platform_device *plat_ci, *phy_pdev;
- struct device_node *phy_np;
+ struct platform_device *plat_ci;
struct resource *res;
struct regulator *reg_vbus;
struct pinctrl *pinctrl;
int ret;
+ struct usb_phy *phy;
if (of_find_property(pdev->dev.of_node, "fsl,usbmisc", NULL)
&& !usbmisc_ops)
@@ -147,19 +146,20 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
return ret;
}
- phy_np = of_parse_phandle(pdev->dev.of_node, "fsl,usbphy", 0);
- if (phy_np) {
- data->phy_np = phy_np;
- phy_pdev = of_find_device_by_node(phy_np);
- if (phy_pdev) {
- struct usb_phy *phy;
- phy = pdev_to_phy(phy_pdev);
- if (phy &&
- try_module_get(phy_pdev->dev.driver->owner)) {
- usb_phy_init(phy);
- data->phy = phy;
- }
+ phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0);
+ if (PTR_ERR(phy) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto err_clk;
+ }
+
+ if (!IS_ERR(phy)) {
+ ret = usb_phy_init(phy);
+ if (ret) {
+ dev_err(&pdev->dev, "unable to init phy: %d\n", ret);
+ goto err_clk;
}
+
+ data->phy = phy;
}
/* we only support host now, so enable vbus here */
@@ -170,7 +170,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
dev_err(&pdev->dev,
"Failed to enable vbus regulator, err=%d\n",
ret);
- goto put_np;
+ goto err_clk;
}
data->reg_vbus = reg_vbus;
} else {
@@ -222,9 +222,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
err:
if (reg_vbus)
regulator_disable(reg_vbus);
-put_np:
- if (phy_np)
- of_node_put(phy_np);
+err_clk:
clk_disable_unprepare(data->clk);
return ret;
}
@@ -244,8 +242,6 @@ static int ci13xxx_imx_remove(struct platform_device *pdev)
module_put(data->phy->dev->driver->owner);
}
- of_node_put(data->phy_np);
-
clk_disable_unprepare(data->clk);
platform_set_drvdata(pdev, NULL);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] USB chipidea i.MX: introduce dr_mode property
From: Sascha Hauer
The dr_mode devicetree property allows to explicitly specify the
host/peripheral/otg mode. This is necessary for boards without proper
ID pin handling.
Reviewed-by: Peter Chen
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
Documentation/devicetree/bindings/usb/ci13xxx-imx.txt |1 +
drivers/usb/chipidea/ci13xxx_imx.c|1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
index dd42ccd..493a414 100644
--- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
+++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
@@ -9,6 +9,7 @@ Recommended properies:
- phy_type: the type of the phy connected to the core. Should be one
of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this
property the PORTSC register won't be touched
+- dr_mode: One of "host", "peripheral" or "otg". Defaults to "otg"
Optional properties:
- fsl,usbphy: phandler of usb phy that connects to the only one port
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index ebc1148..b598bb8 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -114,6 +114,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
CI13XXX_DISABLE_STREAMING;
pdata->phy_mode = of_usb_get_phy_mode(pdev->dev.of_node);
+ pdata->dr_mode = of_usb_get_dr_mode(pdev->dev.of_node);
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
if (!data) {
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] USB chipidea: ci13xxx-imx: create dynamic platformdata
From: Michael Grzeschik
This patch removes the limitation of having only one instance of the
ci13xxx-imx platformdata and makes different configurations possible.
Reviewed-by: Peter Chen
Signed-off-by: Michael Grzeschik
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/chipidea/ci13xxx_imx.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index 8c29122..69024e0 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -85,17 +85,10 @@ EXPORT_SYMBOL_GPL(usbmisc_get_init_data);
/* End of common functions shared by usbmisc drivers*/
-static struct ci13xxx_platform_data ci13xxx_imx_platdata = {
- .name = "ci13xxx_imx",
- .flags = CI13XXX_REQUIRE_TRANSCEIVER |
- CI13XXX_PULLUP_ON_VBUS |
- CI13XXX_DISABLE_STREAMING,
- .capoffset = DEF_CAPOFFSET,
-};
-
static int ci13xxx_imx_probe(struct platform_device *pdev)
{
struct ci13xxx_imx_data *data;
+ struct ci13xxx_platform_data *pdata;
struct platform_device *plat_ci, *phy_pdev;
struct device_node *phy_np;
struct resource *res;
@@ -107,6 +100,18 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
&& !usbmisc_ops)
return -EPROBE_DEFER;
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ dev_err(&pdev->dev, "Failed to allocate CI13xxx-IMX pdata!\n");
+ return -ENOMEM;
+ }
+
+ pdata->name = "ci13xxx_imx";
+ pdata->capoffset = DEF_CAPOFFSET;
+ pdata->flags = CI13XXX_REQUIRE_TRANSCEIVER |
+ CI13XXX_PULLUP_ON_VBUS |
+ CI13XXX_DISABLE_STREAMING;
+
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
if (!data) {
dev_err(&pdev->dev, "Failed to allocate CI13xxx-IMX data!\n");
@@ -168,7 +173,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
reg_vbus = NULL;
}
- ci13xxx_imx_platdata.phy = data->phy;
+ pdata->phy = data->phy;
if (!pdev->dev.dma_mask) {
pdev->dev.dma_mask = devm_kzalloc(&pdev->dev,
@@ -193,7 +198,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
plat_ci = ci13xxx_add_device(&pdev->dev,
pdev->resource, pdev->num_resources,
- &ci13xxx_imx_platdata);
+ pdata);
if (IS_ERR(plat_ci)) {
ret = PTR_ERR(plat_ci);
dev_err(&pdev->dev,
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] USB chipidea: add PTW and PTS handling
From: Michael Grzeschik
This patch makes it possible to configure the PTW and PTS bits inside
the portsc register for host and device mode before the driver starts
and the phy can be addressed as hardware implementation is designed.
Signed-off-by: Michael Grzeschik
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
.../devicetree/bindings/usb/ci13xxx-imx.txt|5 +++
drivers/usb/chipidea/bits.h| 14 ++-
drivers/usb/chipidea/ci13xxx_imx.c |3 ++
drivers/usb/chipidea/core.c| 39
include/linux/usb/chipidea.h |1 +
5 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
index 5778b9c..dd42ccd 100644
--- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
+++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
@@ -5,6 +5,11 @@ Required properties:
- reg: Should contain registers location and length
- interrupts: Should contain controller interrupt
+Recommended properies:
+- phy_type: the type of the phy connected to the core. Should be one
+ of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this
+ property the PORTSC register won't be touched
+
Optional properties:
- fsl,usbphy: phandler of usb phy that connects to the only one port
- fsl,usbmisc: phandler of non-core register device, with one argument
diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
index 050de85..d8ffc2f 100644
--- a/drivers/usb/chipidea/bits.h
+++ b/drivers/usb/chipidea/bits.h
@@ -48,10 +48,22 @@
#define PORTSC_SUSP BIT(7)
#define PORTSC_HSPBIT(9)
#define PORTSC_PTC(0x0FUL << 16)
+/* PTS and PTW for non lpm version only */
+#define PORTSC_PTS(d) d) & 0x3) << 30) | (((d) & 0x4) ? BIT(25) :
0))
+#define PORTSC_PTWBIT(28)
/* DEVLC */
#define DEVLC_PSPD(0x03UL << 25)
-#defineDEVLC_PSPD_HS (0x02UL << 25)
+#define DEVLC_PSPD_HS (0x02UL << 25)
+#define DEVLC_PTW BIT(27)
+#define DEVLC_STS BIT(28)
+#define DEVLC_PTS(d) (((d) & 0x7) << 29)
+
+/* Encoding for DEVLC_PTS and PORTSC_PTS */
+#define PTS_UTMI 0
+#define PTS_ULPI 2
+#define PTS_SERIAL3
+#define PTS_HSIC 4
/* OTGSC */
#define OTGSC_IDPU BIT(5)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index 69024e0..ebc1148 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include "ci.h"
#include "ci13xxx_imx.h"
@@ -112,6 +113,8 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
CI13XXX_PULLUP_ON_VBUS |
CI13XXX_DISABLE_STREAMING;
+ pdata->phy_mode = of_usb_get_phy_mode(pdev->dev.of_node);
+
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
if (!data) {
dev_err(&pdev->dev, "Failed to allocate CI13xxx-IMX data!\n");
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 57cae1f..04d68cb 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -67,6 +67,8 @@
#include
#include
#include
+#include
+#include
#include "ci.h"
#include "udc.h"
@@ -211,6 +213,41 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem
*base)
return 0;
}
+static void hw_phymode_configure(struct ci13xxx *ci)
+{
+ u32 portsc, lpm;
+
+ switch (ci->platdata->phy_mode) {
+ case USBPHY_INTERFACE_MODE_UTMI:
+ portsc = PORTSC_PTS(PTS_UTMI);
+ lpm = DEVLC_PTS(PTS_UTMI);
+ break;
+ case USBPHY_INTERFACE_MODE_UTMIW:
+ portsc = PORTSC_PTS(PTS_UTMI) | PORTSC_PTW;
+ lpm = DEVLC_PTS(PTS_UTMI) | DEVLC_PTW;
+ break;
+ case USBPHY_INTERFACE_MODE_ULPI:
+ portsc = PORTSC_PTS(PTS_ULPI);
+ lpm = DEVLC_PTS(PTS_ULPI);
+ break;
+ case USBPHY_INTERFACE_MODE_SERIAL:
+ portsc = PORTSC_PTS(PTS_SERIAL);
+ lpm = DEVLC_PTS(PTS_SERIAL);
+ break;
+ case USBPHY_INTERFACE_MODE_HSIC:
+ portsc = PORTSC_PTS(PTS_HSIC);
+ lpm = DEVLC_PTS(PTS_HSIC);
+ break;
+ default:
+ return;
+ }
+
+ if (ci->hw_bank.lpm)
+ hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
+ else
+ hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW
[PATCH 3/5] USB chipidea: introduce dual role mode pdata flags
From: Sascha Hauer
Even if a chipidea core is otg capable the board may not. This allows
to explicitly set the core to host/peripheral mode. Without these
flags the driver falls back to the old behaviour.
Signed-off-by: Sascha Hauer
Signed-off-by: Marc Kleine-Budde
---
drivers/usb/chipidea/core.c | 22 --
include/linux/usb/chipidea.h |2 +-
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 04d68cb..ec27060 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -435,6 +435,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
struct resource *res;
void __iomem*base;
int ret;
+ enum usb_dr_mode dr_mode;
if (!dev->platform_data) {
dev_err(dev, "platform data missing\n");
@@ -487,14 +488,23 @@ static int ci_hdrc_probe(struct platform_device *pdev)
return -ENODEV;
}
+ /* For now we treat dual-role as otg */
+ dr_mode = ci->platdata->dr_mode;
+ if (dr_mode == USB_DR_MODE_UNKNOWN || dr_mode == USB_DR_MODE_DUAL_ROLE)
+ dr_mode = USB_DR_MODE_OTG;
+
/* initialize role(s) before the interrupt is requested */
- ret = ci_hdrc_host_init(ci);
- if (ret)
- dev_info(dev, "doesn't support host\n");
+ if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
+ ret = ci_hdrc_host_init(ci);
+ if (ret)
+ dev_info(dev, "doesn't support host\n");
+ }
- ret = ci_hdrc_gadget_init(ci);
- if (ret)
- dev_info(dev, "doesn't support gadget\n");
+ if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
+ ret = ci_hdrc_gadget_init(ci);
+ if (ret)
+ dev_info(dev, "doesn't support gadget\n");
+ }
if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
dev_err(dev, "no supported roles\n");
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 1a2aa18..b314647 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -20,7 +20,7 @@ struct ci13xxx_platform_data {
#define CI13XXX_REQUIRE_TRANSCEIVERBIT(1)
#define CI13XXX_PULLUP_ON_VBUS BIT(2)
#define CI13XXX_DISABLE_STREAMING BIT(3)
-
+ enum usb_dr_modedr_mode;
#define CI13XXX_CONTROLLER_RESET_EVENT 0
#define CI13XXX_CONTROLLER_STOPPED_EVENT 1
void(*notify_event) (struct ci13xxx *ci, unsigned event);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: About chipidea tree
On 02/28/2013 12:16 PM, Alexander Shishkin wrote: > Peter Chen writes: > >> On Tue, Feb 26, 2013 at 02:47:34PM +0100, Marc Kleine-Budde wrote: >>> On 02/26/2013 02:25 PM, Alexander Shishkin wrote: >>>> Marc Kleine-Budde writes: >>>> >>>>> On 02/26/2013 11:56 AM, Alexander Shishkin wrote: >>>>>> Peter Chen writes: >>>>>> >>>>>>> Hi Alex, >>>>>> >>>>>> Hi, >>>>>> >>>>>>> Do we have a chipidea repo which is queued for mainline? >>>>>>> We have several patchsets for chipidea these monthes, I >>>>>>> don't know their status. For me, I would like based >>>>>>> on your tree if it exists. >>>>>> >>>>>> I thought about it, but then it seems like having a separate branch is >>>>>> bound to be confusing to most people. I'd much rather prefer everything >>>>>> go to usb-next, and this is my current plan. Since Greg will start >>>>>> applying new stuff to usb-next after -rc1 is tagged, I'll send my >>>>>> current stash of patches for inclusion then. If your patchset, for >>>>> >>>>> Can we have a look at your queued patches? >>>> >>>> Sure, >>>> http://marc.info/?l=linux-usb&m=135902434508839 >>>> >>>>>> example, has conflicts with my stuff that's not merged, I'll try to take >>>>>> care of resolving the conflicts and submit everything to Greg. In other >>>>>> words, it should be always ok to base your chipidea patchsets on >>>>>> usb-next. >>>>>> >>>>>> Let me know if this sounds reasonable to you. >>>>> >>>>> Michael has already done that work (some S-o-b form Michael are missing) >>>>> and rebased Sascha's and Peter's patches to current linus/master, see >>>>> this tree : >>>>> >>>>> http://git.pengutronix.de/?p=mgr/linux.git;a=shortlog;h=refs/heads/chipidea-for-v3.10 >>>> >>>> Taking a quick look, quite some of those patches are not ready for >>>> inclusion yet. So if the question is, do we need a -next tree for all >>>> the chipidea patchsets floating around, it might be a good thing. But >>>> it's not what Peter was asking in the first place. >>> >>> I suggest that we have a branch that holds all chipidea patches that are >>> ready for mainline. Otherwise it's really hard to code any new features >> >> I agree. >> >> Alex, you can have a repo at github or any other places which is based >> on usb-next, and add it to MAINTAINERS. We can develop the new feature >> based on your repo. Greg can pull it directly if he agrees or you can send >> your queued patchset before every merge windows. > > Ok, let's try this. I have a linux-ci repo on github, might as well do > something useful with it [1], [2]. Currently, the branch called > "ci-for-greg" is where I stack patches that I'm planning to be sending > (via email) to Greg when the time is right. The "policy" is such that > it'll be rebased on top of Greg's usb-next and probably often, so no > fast forwards. Also patches may be dropped from it if necessary. Since > the branch is not for pulling, the "no rebase" rule doesn't apply. > > If you have comments/suggestions/etc for a patch that is in this branch, > please reply to the email with that patch on the mailing list and not > via github infrastructure. > > Sounds reasonable? go ahead. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information
On 02/04/2013 04:58 PM, Roger Quadros wrote:
> The PHY clock, clock rate, VCC regulator and RESET regulator
> can now be provided via device tree.
>
> Signed-off-by: Roger Quadros
> ---
> .../devicetree/bindings/usb/usb-nop-xceiv.txt | 34
>
> drivers/usb/otg/nop-usb-xceiv.c| 31 ++
> 2 files changed, 65 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> new file mode 100644
> index 000..d7e2726
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
> @@ -0,0 +1,34 @@
> +USB NOP PHY
> +
> +Required properties:
> +- compatible: should be usb-nop-xceiv
> +
> +Optional properties:
> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
> + /bindings/clock/clock-bindings.txt
> + This property is required if clock-frequency is specified.
> +
> +- clock-names: Should be "main_clk"
> +
> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
> + be configured to.
> +
> +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
> +
> +- reset-supply: phandle to the regulator that provides power to the PHY.
> +
> +Example:
> +
> + hsusb1_phy {
> + compatible = "usb-nop-xceiv";
> + clock-frequency = <1920>;
Why do you hardcode the clock frequency here? You should use
clk_get_rate() to get the frequency from the clock tree.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information
On 03/08/2013 11:46 AM, Marc Kleine-Budde wrote:
> On 02/04/2013 04:58 PM, Roger Quadros wrote:
>> The PHY clock, clock rate, VCC regulator and RESET regulator
>> can now be provided via device tree.
>>
>> Signed-off-by: Roger Quadros
>> ---
>> .../devicetree/bindings/usb/usb-nop-xceiv.txt | 34
>>
>> drivers/usb/otg/nop-usb-xceiv.c| 31 ++
>> 2 files changed, 65 insertions(+), 0 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>> b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>> new file mode 100644
>> index 000..d7e2726
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>> @@ -0,0 +1,34 @@
>> +USB NOP PHY
>> +
>> +Required properties:
>> +- compatible: should be usb-nop-xceiv
>> +
>> +Optional properties:
>> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
>> + /bindings/clock/clock-bindings.txt
>> + This property is required if clock-frequency is specified.
>> +
>> +- clock-names: Should be "main_clk"
>> +
>> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
>> + be configured to.
>> +
>> +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
>> +
>> +- reset-supply: phandle to the regulator that provides power to the PHY.
>> +
>> +Example:
>> +
>> +hsusb1_phy {
>> +compatible = "usb-nop-xceiv";
>> +clock-frequency = <1920>;
>
> Why do you hardcode the clock frequency here? You should use
> clk_get_rate() to get the frequency from the clock tree.
What about declaring a "fixed-clock" node in the device tree? Then it
should be possible to keep the driver free of any omap specific code.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH 3/4] USB mxs-phy: use readl(), writel() instead of the _relaxed() versions
On 02/28/2013 12:25 PM, Alexander Shishkin wrote: > Marc Kleine-Budde writes: > >> This patch converts the mxs-phy driver from readl_relaxed(), writel_relaxed() >> to the plain readl(), writel() functions, which are available on all >> platforms. >> This is done to enable compile time testing on non ARM platforms. >> >> Reported-by: Alexander Shishkin > > I think it was Felipe who reported this actually. Fixed. I'm about to repost this series with crediting Felipe as reporter. Which tree would you like to be this series based on? It depends on the "USB otg: use try_module_get in all usb_get_phy functions and add missing module_put" patch which is part of your usb/fixes branch, but not in usb/next yet. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH 3/4] USB mxs-phy: use readl(), writel() instead of the _relaxed() versions
On 03/08/2013 05:28 PM, Felipe Balbi wrote: > Hi, > > On Fri, Mar 08, 2013 at 05:05:58PM +0100, Marc Kleine-Budde wrote: >> On 02/28/2013 12:25 PM, Alexander Shishkin wrote: >>> Marc Kleine-Budde writes: >>> >>>> This patch converts the mxs-phy driver from readl_relaxed(), >>>> writel_relaxed() >>>> to the plain readl(), writel() functions, which are available on all >>>> platforms. >>>> This is done to enable compile time testing on non ARM platforms. >>>> >>>> Reported-by: Alexander Shishkin >>> >>> I think it was Felipe who reported this actually. >> >> Fixed. I'm about to repost this series with crediting Felipe as reporter. >> >> Which tree would you like to be this series based on? It depends on the >> "USB otg: use try_module_get in all usb_get_phy functions and add >> missing module_put" patch which is part of your usb/fixes branch, but >> not in usb/next yet. > > look at my testing branch. I have them all already. You didn't get an > email yet because I'm waiting for -rc2 to be tagged. Thanks. > I have fixed the Reported-by tag, no issues there. > > Anyway, as soon as v3.9-rc2 is tagged, I will rebase testing on top of > that and move all patches to 'next', then everything will be stable from > that point on. > > If you want to have changes to any of the patches, now is the time, > after I move patches to 'next' the tree becomes immutable. Nope, I just wanted to keep track of the patches. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH 5/5] USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy
On 03/08/2013 05:33 PM, Felipe Balbi wrote:
> On Thu, Feb 28, 2013 at 11:57:04AM +0100, Marc Kleine-Budde wrote:
>> @@ -147,19 +146,20 @@ static int ci13xxx_imx_probe(struct platform_device
>> *pdev)
>> return ret;
>> }
>>
>> -phy_np = of_parse_phandle(pdev->dev.of_node, "fsl,usbphy", 0);
>> -if (phy_np) {
>> -data->phy_np = phy_np;
>> -phy_pdev = of_find_device_by_node(phy_np);
>> -if (phy_pdev) {
>> -struct usb_phy *phy;
>> -phy = pdev_to_phy(phy_pdev);
>> -if (phy &&
>> -try_module_get(phy_pdev->dev.driver->owner)) {
>> -usb_phy_init(phy);
>> -data->phy = phy;
>> -}
>> +phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0);
>
> very nice, but should be done at chipidea core.
Any suggestions for the phandle name? "chipidea,usbphy"?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH 3/5] USB chipidea: introduce dual role mode pdata flags
On 03/08/2013 05:46 PM, Alexander Shishkin wrote:
> On 8 March 2013 18:33, Felipe Balbi wrote:
>> Hi,
>>
>> On Thu, Feb 28, 2013 at 11:57:02AM +0100, Marc Kleine-Budde wrote:
>>> @@ -487,14 +488,23 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>>> return -ENODEV;
>>> }
>>>
>>> + /* For now we treat dual-role as otg */
>>> + dr_mode = ci->platdata->dr_mode;
>>> + if (dr_mode == USB_DR_MODE_UNKNOWN || dr_mode ==
>>> USB_DR_MODE_DUAL_ROLE)
>>> + dr_mode = USB_DR_MODE_OTG;
>>> +
>>> /* initialize role(s) before the interrupt is requested */
>>> - ret = ci_hdrc_host_init(ci);
>>> - if (ret)
>>> - dev_info(dev, "doesn't support host\n");
>>> + if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
>>
>> this is not something you should be passing via pdata; chipidea core
>> should know how to read this data by itself. Meaning that chipidea core
>> should be taught about devicetree. But make it optional since now all
>> users use DT.
>
> And I don't think I like the idea of chipidea core calling into device
> tree code directly.
Hmmmthis means draw :)
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
