From: Peter Chen <hzpeterchen@gmail.com>
To: rogerq@ti.com
Cc: pawell@cadence.com, devicetree@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-usb@vger.kernel.org, lkml <linux-kernel@vger.kernel.org>,
adouglas@cadence.com, jbergsagel@ti.com, nsekhar@ti.com,
nm@ti.com, sureshp@cadence.com, peter.chen@nxp.com,
pjez@cadence.com, kurahul@cadence.com
Subject: Re: [RFC PATCH v2 04/15] usb:cdns3: Driver initialization code.
Date: Wed, 5 Dec 2018 16:57:12 +0800 [thread overview]
Message-ID: <CAL411-rdMjjWBB0sKV690kpGdqiTPpMTaKsJ6aEaABgzig=cFw@mail.gmail.com> (raw)
In-Reply-To: <5C065AEE.4010205@ti.com>
>
>
>
> On 04/12/18 10:50, Peter Chen wrote:
> >>> + * Cadence USBSS DRD Driver.
> >>> + *
> >>> + * Copyright (C) 2018 Cadence.
> >>> + *
> >>> + * Author: Peter Chen <peter.chen@nxp.com>
> >>> + * Pawel Laszczak <pawell@cadence.com>
> >>> + */
> >>> +
> >>> +#include <linux/module.h>
> >>> +#include <linux/kernel.h>
> >>> +#include <linux/platform_device.h>
> >>> +#include <linux/interrupt.h>
> >>> +#include <linux/io.h>
> >>> +#include <linux/pm_runtime.h>
> >>> +
> >>> +#include "gadget.h"
> >>> +#include "core.h"
> >>> +
> >>> +static inline struct cdns3_role_driver *cdns3_get_current_role_driver(struct cdns3 *cdns)
> >>> +{
> >>> + WARN_ON(cdns->role >= CDNS3_ROLE_END || !cdns->roles[cdns->role]);
> >>> + return cdns->roles[cdns->role];
> >>> +}
> >>> +
> >>> +static inline int cdns3_role_start(struct cdns3 *cdns, enum cdns3_roles role)
> >>> +{
> >>> + int ret;
> >>> +
> >>> + if (role >= CDNS3_ROLE_END)
> >>
> >> WARN_ON()?
> >>
> >>> + return 0;
> >>> +
> >>> + if (!cdns->roles[role])
> >>> + return -ENXIO;
> >>> +
> >>> + mutex_lock(&cdns->mutex);
> >>> + cdns->role = role;
> >>> + ret = cdns->roles[role]->start(cdns);
> >>> + mutex_unlock(&cdns->mutex);
> >>> + return ret;
> >>> +}
> >>> +
> >>> +static inline void cdns3_role_stop(struct cdns3 *cdns)
> >>> +{
> >>> + enum cdns3_roles role = cdns->role;
> >>> +
> >>> + if (role == CDNS3_ROLE_END)
> >>
> >> WARN_ON(role >= CNDS3_ROLE_END) ?
> >>
> >>> + return;
> >>> +
> >>> + mutex_lock(&cdns->mutex);
> >>> + cdns->roles[role]->stop(cdns);
> >>> + cdns->role = CDNS3_ROLE_END;
> >>
> >> Why change the role here? You are just stopping the role not changing it.
> >> I think cdns->role should remain unchanged, so we can call cdns3_role_start()
> >> if required without error.
> >>
> >
> > The current version of this IP has some issues to detect vbus status correctly,
> > we have to force vbus status accordingly, so we need a status to indicate
> > vbus disconnection, and add some code to let controller know vbus
> > removal, in that case, the controller's state machine can be correct.
> > So, we increase one role 'CDNS3_ROLE_END' to for this purpose.
> >
> > CDNS3_ROLE_GADGET: gadget mode and VBUS on
> > CDNS3_ROLE_HOST: host mode and VBUS on
> > CDNS3_ROLE_END: VBUS off, eg either host or device cable on the port.
> >
> > So, we may start role from CDNS3_ROLE_END at probe when nothing is connected,
> > and need to set role as CDNS3_ROLE_END at ->stop for further handling at
> > role switch routine.
>
> OK. but still this (changing to ROLE_END) must be moved to the role switch routine
> and the explanation you just mentioned must be added there.
>
If we use host part as separate driver, something may need to change.
The ROLE_END
may need to add at role switch routine as your said.
> >
> >>> + mutex_unlock(&cdns->mutex);
> >>> +}
> >>> +
> >>> +static enum cdns3_roles cdns3_get_role(struct cdns3 *cdns)
> >>> +{
> >>> + if (cdns->roles[CDNS3_ROLE_HOST] && cdns->roles[CDNS3_ROLE_GADGET]) {
> >>> + //TODO: implements selecting device/host mode
> >>> + return CDNS3_ROLE_HOST;
> >>> + }
> >>> + return cdns->roles[CDNS3_ROLE_HOST]
> >>> + ? CDNS3_ROLE_HOST
> >>> + : CDNS3_ROLE_GADGET;
> >>
> >> Why not just
> >> return cdns->role;
> >>
> >> I'm wondering if we really need this function.
> >
> > cdns->role gets from cdns3_get_role, and this API tells role at the runtime.
> > If both roles are supported, the role is decided by external
> > conditions, eg, vbus/id
> > or external connector. If only single role is supported, only one role structure
> > is allocated, cdns->roles[CDNS3_ROLE_HOST] or cdns->roles[CDNS3_ROLE_GADGET]
> >
>
> How about adding this description in function documentation.
>
Sure, but may need to redesign it due to host part as a standalone driver.
> >>
> >> Then you can move cdns3_role_start() as well to core_init_role().
> >>
> >
> > Usually, we request irq after hardware initialization has finished, if not,
> > there may unexpected interrupt.
>
> Doesn't kernel warn if interrupt happens and there is no handler?
> To avoid that I was suggesting to request_irq first.
>
The interrupt will not happen (related bit at GIC is masked) until we
call request_irq.
And we don't want to handle interrupts until hardware initialization, don't we?
Peter
next prev parent reply other threads:[~2018-12-05 8:57 UTC|newest]
Thread overview: 85+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-18 10:08 [RFC PATCH v2 00/15] Introduced new Cadence USBSS DRD Driver Pawel Laszczak
2018-11-18 10:08 ` [RFC PATCH v2 01/15] usb:cdns3: add pci to platform driver wrapper Pawel Laszczak
2018-11-23 10:44 ` Roger Quadros
2018-11-18 10:08 ` [RFC PATCH v2 02/15] usb:cdns3: Device side header file Pawel Laszczak
2018-11-30 6:48 ` PETER CHEN
2018-12-02 19:27 ` Pawel Laszczak
2018-11-18 10:08 ` [RFC PATCH v2 03/15] dt-bindings: add binding for USBSS-DRD controller Pawel Laszczak
2018-11-23 10:53 ` Roger Quadros
2018-11-25 7:33 ` Pawel Laszczak
2018-12-04 22:41 ` Rob Herring
2018-12-06 10:26 ` Pawel Laszczak
2018-11-18 10:09 ` [RFC PATCH v2 04/15] usb:cdns3: Driver initialization code Pawel Laszczak
2018-11-23 11:35 ` Roger Quadros
2018-11-25 12:35 ` Pawel Laszczak
2018-12-04 9:09 ` Peter Chen
2018-12-06 7:00 ` Pawel Laszczak
2018-12-04 8:50 ` Peter Chen
2018-12-04 10:46 ` Roger Quadros
2018-12-05 8:57 ` Peter Chen [this message]
2018-12-06 9:31 ` Pawel Laszczak
2018-12-05 19:24 ` Pawel Laszczak
2018-12-05 19:42 ` Pawel Laszczak
2018-12-06 10:02 ` Pawel Laszczak
2018-11-30 7:32 ` Peter Chen
2018-12-02 20:34 ` Pawel Laszczak
2018-12-04 7:11 ` Peter Chen
2018-12-05 7:19 ` Pawel Laszczak
2018-12-05 8:55 ` Alan Douglas
2018-12-05 9:07 ` Peter Chen
2018-11-18 10:09 ` [RFC PATCH v2 05/15] usb:cdns3: Added DRD support Pawel Laszczak
2018-11-23 14:51 ` Roger Quadros
2018-11-26 7:23 ` Pawel Laszczak
2018-11-26 8:07 ` Roger Quadros
2018-11-26 8:39 ` Pawel Laszczak
2018-11-26 9:39 ` Roger Quadros
2018-11-26 10:09 ` Pawel Laszczak
2018-11-26 10:15 ` Roger Quadros
2018-11-27 11:29 ` Pawel Laszczak
2018-11-27 12:10 ` Roger Quadros
2018-12-04 9:18 ` Peter Chen
2018-12-06 7:25 ` Pawel Laszczak
2018-11-18 10:09 ` [RFC PATCH v2 06/15] usb:cdns3: Adds Host support Pawel Laszczak
2018-11-23 14:23 ` Roger Quadros
2018-11-26 8:24 ` Pawel Laszczak
2018-11-26 9:50 ` Roger Quadros
2018-11-26 10:17 ` Pawel Laszczak
2018-12-05 8:41 ` Peter Chen
2018-11-18 10:09 ` [RFC PATCH v2 07/15] usb:cdns3: Adds Device mode support - initialization Pawel Laszczak
2018-11-28 11:34 ` Roger Quadros
2018-11-28 11:40 ` Felipe Balbi
2018-11-30 4:20 ` PETER CHEN
2018-11-30 6:29 ` Pawel Laszczak
2018-11-30 14:36 ` Pawel Laszczak
2018-11-18 10:09 ` [RFC PATCH v2 08/15] usb:cdns3: Implements device operations part of the API Pawel Laszczak
2018-11-28 12:22 ` Roger Quadros
2018-12-01 11:11 ` Pawel Laszczak
2018-12-03 10:19 ` Pawel Laszczak
2018-12-10 2:12 ` Peter Chen
2018-12-11 11:26 ` Sekhar Nori
2018-12-11 19:49 ` Pawel Laszczak
2018-12-14 1:34 ` Peter Chen
2018-12-14 6:49 ` Pawel Laszczak
2018-12-14 10:39 ` Sekhar Nori
2018-12-14 10:47 ` Felipe Balbi
2018-12-14 11:13 ` Sekhar Nori
2018-12-14 11:26 ` Felipe Balbi
2018-12-14 12:20 ` Sekhar Nori
2018-12-14 12:30 ` Felipe Balbi
2018-12-16 13:31 ` Pawel Laszczak
2018-11-18 10:09 ` [RFC PATCH v2 09/15] usb:cdns3: EpX " Pawel Laszczak
2018-11-28 12:46 ` Roger Quadros
2018-12-01 13:30 ` Pawel Laszczak
2018-11-18 10:09 ` [RFC PATCH v2 10/15] usb:cdns3: Ep0 " Pawel Laszczak
2018-11-28 14:31 ` Roger Quadros
2018-12-02 10:34 ` Pawel Laszczak
2018-11-18 10:09 ` [RFC PATCH v2 11/15] usb:cdns3: Implements ISR functionality Pawel Laszczak
2018-11-28 14:54 ` Roger Quadros
2018-12-02 11:49 ` Pawel Laszczak
2018-12-02 12:52 ` Pawel Laszczak
2018-11-18 10:09 ` [RFC PATCH v2 12/15] usb:cdns3: Adds enumeration related function Pawel Laszczak
2018-11-28 15:50 ` Roger Quadros
2018-12-02 16:39 ` Pawel Laszczak
2018-11-18 10:09 ` [RFC PATCH v2 13/15] usb:cdns3: Adds transfer " Pawel Laszczak
2018-11-18 10:09 ` [RFC PATCH v2 14/15] usb:cdns3: Adds debugging function Pawel Laszczak
2018-11-18 10:09 ` [RFC PATCH v2 15/15] usb:cdns3: Feature for changing role Pawel Laszczak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAL411-rdMjjWBB0sKV690kpGdqiTPpMTaKsJ6aEaABgzig=cFw@mail.gmail.com' \
--to=hzpeterchen@gmail.com \
--cc=adouglas@cadence.com \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=jbergsagel@ti.com \
--cc=kurahul@cadence.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=nm@ti.com \
--cc=nsekhar@ti.com \
--cc=pawell@cadence.com \
--cc=peter.chen@nxp.com \
--cc=pjez@cadence.com \
--cc=rogerq@ti.com \
--cc=sureshp@cadence.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).