Re: [PATCH v2 1/3] staging: typec: fusb302: Rename fcs,extcon-name to linux,extcon-name
On Wed, Dec 05, 2018 at 10:25:36AM +0100, Greg Kroah-Hartman wrote: > On Mon, Dec 03, 2018 at 09:23:44PM +0200, Andy Shevchenko wrote: > > On Thu, Nov 15, 2018 at 04:53:03PM +0200, Heikki Krogerus wrote: > > > FWIW, the series: > > > Reviewed-by: Heikki Krogerus > > > > Greg, can you pick up this patch (first in the series, since dwc3 went > > through Felipe's tree)? > > Sorry for the delay, now queued up. NP, thanks! -- With Best Regards, Andy Shevchenko
Re: [PATCH v1] usb: dwc3: trace: Refactor nested switch to make compiler happy
On Wed, Dec 05, 2018 at 11:18:45AM +0200, Andy Shevchenko wrote:
> On Wed, Dec 05, 2018 at 11:10:46AM +0200, Felipe Balbi wrote:
> > Andy Shevchenko writes:
> >
> > > The missed break statement in the outer switch makes the code fall through
> > > always and thus always same value will be printed.
> > >
> > > Besides that, compiler warns about missed fall through marker:
> > >
> > > drivers/usb/dwc3/./trace.h: In function ‘trace_raw_output_dwc3_log_trb’:
> > > drivers/usb/dwc3/./trace.h:246:4: warning: this statement may fall
> > > through [-Wimplicit-fallthrough=]
> > > switch (pcm) {
> > > ^~
> > easier to add "break" here, no? That would be the minimal fix.
>
> No. Then you would need to add same default to the inner switch.
Ah, you meant that pcm would be never outside of the given cases.
Yes, that's fine then, consider my patch as a bugreport.
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v1] usb: dwc3: trace: Refactor nested switch to make compiler happy
On Wed, Dec 05, 2018 at 11:10:46AM +0200, Felipe Balbi wrote:
> Andy Shevchenko writes:
>
> > The missed break statement in the outer switch makes the code fall through
> > always and thus always same value will be printed.
> >
> > Besides that, compiler warns about missed fall through marker:
> >
> > drivers/usb/dwc3/./trace.h: In function ‘trace_raw_output_dwc3_log_trb’:
> > drivers/usb/dwc3/./trace.h:246:4: warning: this statement may fall through
> > [-Wimplicit-fallthrough=]
> > switch (pcm) {
> > ^~
> >
> > Refactor nested switch statements to work correctly without
> > compilation warnings.
> >
> > Fixes: fa8d965d736b ("usb: dwc3: trace: pretty print high-bandwidth
> > transfers too")
> > Cc: Felipe Balbi
> > Signed-off-by: Andy Shevchenko
> > ---
> > drivers/usb/dwc3/trace.h | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h
> > index f22714cce070..8e1625a6c19f 100644
> > --- a/drivers/usb/dwc3/trace.h
> > +++ b/drivers/usb/dwc3/trace.h
> > @@ -238,7 +238,7 @@ DECLARE_EVENT_CLASS(dwc3_log_trb,
> > ),
> > TP_printk("%s: trb %p buf %08x%08x size %s%d ctrl %08x
> > (%c%c%c%c:%c%c:%s)",
> > __get_str(name), __entry->trb, __entry->bph, __entry->bpl,
> > - ({char *s;
> > + ({ char *s = "";
> > int pcm = ((__entry->size >> 24) & 3) + 1;
> > switch (__entry->type) {
> > case USB_ENDPOINT_XFER_INT:
> > @@ -254,8 +254,6 @@ DECLARE_EVENT_CLASS(dwc3_log_trb,
> > s = "3x ";
> > break;
> > }
>
> easier to add "break" here, no? That would be the minimal fix.
No. Then you would need to add same default to the inner switch.
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 1/3] staging: typec: fusb302: Rename fcs,extcon-name to linux,extcon-name
On Thu, Nov 15, 2018 at 04:53:03PM +0200, Heikki Krogerus wrote: > On Thu, Nov 15, 2018 at 03:16:19PM +0200, Andy Shevchenko wrote: > > Since we are going to use the same in Designware USB 3 driver, > > rename the property to be consistent across the drivers. > > > > No functional change intended. > > > > Signed-off-by: Andy Shevchenko > > Cc: Hans de Goede > > Cc: Guenter Roeck > > Acked-by: Hans de Goede > > Acked-by: Guenter Roeck > > FWIW, the series: > Reviewed-by: Heikki Krogerus Greg, can you pick up this patch (first in the series, since dwc3 went through Felipe's tree)? -- With Best Regards, Andy Shevchenko
[PATCH v1] usb: dwc3: trace: Refactor nested switch to make compiler happy
The missed break statement in the outer switch makes the code fall through
always and thus always same value will be printed.
Besides that, compiler warns about missed fall through marker:
drivers/usb/dwc3/./trace.h: In function ‘trace_raw_output_dwc3_log_trb’:
drivers/usb/dwc3/./trace.h:246:4: warning: this statement may fall through
[-Wimplicit-fallthrough=]
switch (pcm) {
^~
Refactor nested switch statements to work correctly without
compilation warnings.
Fixes: fa8d965d736b ("usb: dwc3: trace: pretty print high-bandwidth transfers
too")
Cc: Felipe Balbi
Signed-off-by: Andy Shevchenko
---
drivers/usb/dwc3/trace.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h
index f22714cce070..8e1625a6c19f 100644
--- a/drivers/usb/dwc3/trace.h
+++ b/drivers/usb/dwc3/trace.h
@@ -238,7 +238,7 @@ DECLARE_EVENT_CLASS(dwc3_log_trb,
),
TP_printk("%s: trb %p buf %08x%08x size %s%d ctrl %08x
(%c%c%c%c:%c%c:%s)",
__get_str(name), __entry->trb, __entry->bph, __entry->bpl,
- ({char *s;
+ ({ char *s = "";
int pcm = ((__entry->size >> 24) & 3) + 1;
switch (__entry->type) {
case USB_ENDPOINT_XFER_INT:
@@ -254,8 +254,6 @@ DECLARE_EVENT_CLASS(dwc3_log_trb,
s = "3x ";
break;
}
- default:
- s = "";
} s; }),
DWC3_TRB_SIZE_LENGTH(__entry->size), __entry->ctrl,
__entry->ctrl & DWC3_TRB_CTRL_HWO ? 'H' : 'h',
--
2.19.2
[PATCH v2 2/3] usb: dwc3: drd: Switch to device property for 'extcon' handling
Switch to device property for 'extcon' handling. No functional change intended. Signed-off-by: Andy Shevchenko Acked-by: Hans de Goede --- drivers/usb/dwc3/drd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c index 218371f985ca..2401bd504891 100644 --- a/drivers/usb/dwc3/drd.c +++ b/drivers/usb/dwc3/drd.c @@ -10,6 +10,7 @@ #include #include #include +#include #include "debug.h" #include "core.h" @@ -446,8 +447,8 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc) struct device_node *np_phy, *np_conn; struct extcon_dev *edev; - if (of_property_read_bool(dev->of_node, "extcon")) - return extcon_get_edev_by_phandle(dwc->dev, 0); + if (device_property_read_bool(dev, "extcon")) + return extcon_get_edev_by_phandle(dev, 0); np_phy = of_parse_phandle(dev->of_node, "phys", 0); np_conn = of_graph_get_remote_node(np_phy, -1, -1); -- 2.19.1
[PATCH v2 3/3] usb: dwc3: drd: Add support for DR detection through extcon
Allow extcon device, found by name, to provide DR status for USB.
This is needed, for example, in case of Intel Merrifield platform,
where the Intel Basin Cove PMIC provides an extcon device to communicate
the detected role.
Note, that the "linux,extcon-name" property name is only for kernel
internal use by X86/ACPI platform code and as such is not documented
in the device tree bindings.
Signed-off-by: Andy Shevchenko
Acked-by: Hans de Goede
---
drivers/usb/dwc3/drd.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 2401bd504891..726100d1ac0d 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -446,10 +446,25 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3
*dwc)
struct device *dev = dwc->dev;
struct device_node *np_phy, *np_conn;
struct extcon_dev *edev;
+ const char *name;
if (device_property_read_bool(dev, "extcon"))
return extcon_get_edev_by_phandle(dev, 0);
+ /*
+* Device tree platforms should get extcon via phandle.
+* On ACPI platforms, we get the name from a device property.
+* This device property is for kernel internal use only and
+* is expected to be set by the glue code.
+*/
+ if (device_property_read_string(dev, "linux,extcon-name", ) == 0) {
+ edev = extcon_get_extcon_dev(name);
+ if (!edev)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ return edev;
+ }
+
np_phy = of_parse_phandle(dev->of_node, "phys", 0);
np_conn = of_graph_get_remote_node(np_phy, -1, -1);
--
2.19.1
[PATCH v2 1/3] staging: typec: fusb302: Rename fcs,extcon-name to linux,extcon-name
Since we are going to use the same in Designware USB 3 driver,
rename the property to be consistent across the drivers.
No functional change intended.
Signed-off-by: Andy Shevchenko
Cc: Hans de Goede
Cc: Guenter Roeck
Acked-by: Hans de Goede
Acked-by: Guenter Roeck
---
drivers/platform/x86/intel_cht_int33fe.c | 2 +-
drivers/usb/typec/tcpm/fusb302.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/intel_cht_int33fe.c
b/drivers/platform/x86/intel_cht_int33fe.c
index 464fe93657b5..87cbf18d0305 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -79,7 +79,7 @@ static const struct property_entry max17047_props[] = {
};
static const struct property_entry fusb302_props[] = {
- PROPERTY_ENTRY_STRING("fcs,extcon-name", "cht_wcove_pwrsrc"),
+ PROPERTY_ENTRY_STRING("linux,extcon-name", "cht_wcove_pwrsrc"),
PROPERTY_ENTRY_U32("fcs,max-sink-microvolt", 1200),
PROPERTY_ENTRY_U32("fcs,max-sink-microamp", 300),
PROPERTY_ENTRY_U32("fcs,max-sink-microwatt", 3600),
diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index 43b64d9309d0..e9344997329c 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -1765,7 +1765,7 @@ static int fusb302_probe(struct i2c_client *client,
* to be set by the platform code which also registers the i2c client
* for the fusb302.
*/
- if (device_property_read_string(dev, "fcs,extcon-name", ) == 0) {
+ if (device_property_read_string(dev, "linux,extcon-name", ) == 0) {
chip->extcon = extcon_get_extcon_dev(name);
if (!chip->extcon)
return -EPROBE_DEFER;
--
2.19.1
Re: [PATCH v15 0/2] Add support for USB Type-C interface on latest NVIDIA GPU
On Fri, Nov 09, 2018 at 05:45:16PM +0100, Wolfram Sang wrote: > On Mon, Oct 29, 2018 at 02:42:37PM +0200, Andy Shevchenko wrote: > > On Fri, Oct 26, 2018 at 09:36:57AM -0700, Ajay Gupta wrote: > > > Hi Heikki and Wolfram, > > > > > > These two changes add support for USB Type-C interface on latest NVIDIA > > > GPU card. > > > The Type-C controller used is Cypress CCGx and is over I2C interface. > > > > > > I2C host controller has known limitation of sending STOP after every > > > read. Since > > > each read can be of 4 byte maximum length so there is a limit of 4 byte > > > read. > > > This is mentioned in adapter quirks as "max_read_len = 4" > > > > > > I2C host controller is mainly used for "write-then-read" or "write" > > > messages so added > > > the flag I2C_AQ_COMB_WRITE_THEN_READ in adapter quirks. > > > > > > PATCH[2/2] on ucsi driver now have added logic to check i2c adapter > > > quirks and > > > issues i2c read transfer based on max_read_len quirk settings. This will > > > make sure > > > the read limitation is not affecting I2C host which do not have such > > > limitation. > > > > > > I think the patches should through usb tree because the main > > > functionality is > > > usb Type-C. > > > > FWIW, > > Reviewed-by: Andy Shevchenko > > Thanks, Andy. Your review is worth something :) > > May I ask you to add your tag to the individual patches (as long as it > is not a patch bomb), so patchwork will pick them up and I see them > immediately? Hmm... It would be a bit difficult (requires to download patches again as mbox). I would rather give you a hint how I resolve such problem when apply a tag to a pile of patches. Pre-requisites: - git branch with all patches in question located at the top (let's assume 2 patches) You may just run the following: git filter-branch --msg-filter \ 'sed -e "/Signed-off-by:/ a Reviewed-by: Andy Shevchenko "' \ -f HEAD~2..HEAD It will append a given line after first match with SoB one (`man sed` for details). Check without -f (which enforces to rewrite) first. -- With Best Regards, Andy Shevchenko
Re: [PATCH v15 0/2] Add support for USB Type-C interface on latest NVIDIA GPU
On Fri, Oct 26, 2018 at 09:36:57AM -0700, Ajay Gupta wrote: > Hi Heikki and Wolfram, > > These two changes add support for USB Type-C interface on latest NVIDIA GPU > card. > The Type-C controller used is Cypress CCGx and is over I2C interface. > > I2C host controller has known limitation of sending STOP after every read. > Since > each read can be of 4 byte maximum length so there is a limit of 4 byte read. > This is mentioned in adapter quirks as "max_read_len = 4" > > I2C host controller is mainly used for "write-then-read" or "write" messages > so added > the flag I2C_AQ_COMB_WRITE_THEN_READ in adapter quirks. > > PATCH[2/2] on ucsi driver now have added logic to check i2c adapter quirks and > issues i2c read transfer based on max_read_len quirk settings. This will make > sure > the read limitation is not affecting I2C host which do not have such > limitation. > > I think the patches should through usb tree because the main functionality is > usb Type-C. FWIW, Reviewed-by: Andy Shevchenko > > Thanks > Ajay > > Ajay Gupta (2): > i2c: buses: add i2c bus driver for NVIDIA GPU > usb: typec: ucsi: add support for Cypress CCGx > > Documentation/i2c/busses/i2c-nvidia-gpu | 18 ++ > MAINTAINERS | 7 + > drivers/i2c/busses/Kconfig | 9 + > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-nvidia-gpu.c | 368 > drivers/usb/typec/ucsi/Kconfig | 10 + > drivers/usb/typec/ucsi/Makefile | 2 + > drivers/usb/typec/ucsi/ucsi_ccg.c | 307 > 8 files changed, 722 insertions(+) > create mode 100644 Documentation/i2c/busses/i2c-nvidia-gpu > create mode 100644 drivers/i2c/busses/i2c-nvidia-gpu.c > create mode 100644 drivers/usb/typec/ucsi/ucsi_ccg.c > > -- > 2.17.1 > -- With Best Regards, Andy Shevchenko
Re: [PATCH v14 2/2] usb: typec: ucsi: add support for Cypress CCGx
On Thu, Oct 25, 2018 at 03:33:53PM -0700, aj...@nvidia.com wrote:
> Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller
> over I2C interface.
>
> This UCSI I2C driver uses I2C bus driver interface for communicating
> with Type-C controller.
> + /*
> + * Flush CCGx RESPONSE queue by acking interrupts. Above ucsi control
> + * register write will push response which must be cleared.
> + */
> + do {
> + status = ccg_read(uc, CCGX_RAB_INTR_REG, , sizeof(data));
> + if (status < 0)
> + return status;
> +
> + if (!data)
> + return 0;
> +
> + status = ccg_write(uc, CCGX_RAB_INTR_REG, , sizeof(data));
> + if (status < 0)
> + return status;
> +
> + usleep_range(1, 11000);
> + } while (data && --count);
I don't see any point to check data here. How can it be different from the
check above?
> + return -ETIMEDOUT;
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v13 2/2] usb: typec: ucsi: add support for Cypress CCGx
On Wed, Oct 24, 2018 at 05:43:27PM +, Ajay Gupta wrote:
> Hi Andy
>
> > > > > Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller over
> > > > > I2C interface.
> > > > >
> > > > > This UCSI I2C driver uses I2C bus driver interface for
> > > > > communicating with Type-C controller.
> >
> > > > > + /*
> > > > > + * Flush CCGx RESPONSE queue by acking interrupts. Above ucsi
> > > > control
> > > > > + * register write will push response which must be cleared.
> > > > > + */
> > > > > + status = ccg_read(uc, CCGX_RAB_INTR_REG, , sizeof(data));
> > > > > + if (status < 0)
> > > > > + return status;
> >
> > (1)
> >
> > > > > + do {
> > > > > + status = ccg_write(uc, CCGX_RAB_INTR_REG, ,
> > > > sizeof(data));
> > > > > + if (status < 0)
> > > > > + return status;
> >
> > (2)
> >
> > > > > +
> > > > > + usleep_range(1, 11000);
> > > > > +
> > > > > + status = ccg_read(uc, CCGX_RAB_INTR_REG, ,
> > > > sizeof(data));
> > > > > + if (status < 0)
> > > > > + return status;
> > > > > + } while ((data != 0x00) && count--);
> > > >
> > > > What's the significance of that count?
> > > It is like a retry count to clear interrupt status.
> > >
> > > > Shouldn't you return -ETIMEDOUT if count == 0?
> > > Yes. Good catch. Does the below fix looks ok?
> >
> > At least for me looks OK (I dunno why I missed that what Heikki found
> > recently).
> > Nevertheless, I have one more question about (1) and (2) above.
> >
> > Is it necessary to do one more read before do-while loop?
> Yes, we need to read to get interrupt status bits and then write
> them back to clear the status.
Ah, I see, but why you not reorganize it to put this into do-while loop?
do {
read
write
check for data
sleep
} while (--count);
Also note predecrement.
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v13 2/2] usb: typec: ucsi: add support for Cypress CCGx
On Tue, Oct 23, 2018 at 06:56:59PM +, Ajay Gupta wrote:
> > On Wed, Oct 03, 2018 at 11:27:28AM -0700, Ajay Gupta wrote:
> > > Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller over I2C
> > > interface.
> > >
> > > This UCSI I2C driver uses I2C bus driver interface for communicating
> > > with Type-C controller.
> > > + /*
> > > + * Flush CCGx RESPONSE queue by acking interrupts. Above ucsi
> > control
> > > + * register write will push response which must be cleared.
> > > + */
> > > + status = ccg_read(uc, CCGX_RAB_INTR_REG, , sizeof(data));
> > > + if (status < 0)
> > > + return status;
(1)
> > > + do {
> > > + status = ccg_write(uc, CCGX_RAB_INTR_REG, ,
> > sizeof(data));
> > > + if (status < 0)
> > > + return status;
(2)
> > > +
> > > + usleep_range(1, 11000);
> > > +
> > > + status = ccg_read(uc, CCGX_RAB_INTR_REG, ,
> > sizeof(data));
> > > + if (status < 0)
> > > + return status;
> > > + } while ((data != 0x00) && count--);
> >
> > What's the significance of that count?
> It is like a retry count to clear interrupt status.
>
> > Shouldn't you return -ETIMEDOUT if count == 0?
> Yes. Good catch. Does the below fix looks ok?
At least for me looks OK (I dunno why I missed that what Heikki found recently).
Nevertheless, I have one more question about (1) and (2) above.
Is it necessary to do one more read before do-while loop?
>
> do {
> status = ccg_write(uc, CCGX_RAB_INTR_REG, ,
> sizeof(data));
> if (status < 0)
> return status;
>
> usleep_range(1, 11000);
>
> status = ccg_read(uc, CCGX_RAB_INTR_REG, , sizeof(data));
> if (status < 0)
> return status;
>
> if (!data)
> return 0;
> } while (data && count--);
>
> return -ETIMEDOUT;
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v6 2/2] usb: typec: ucsi: add support for Cypress CCGx
On Thu, Sep 6, 2018 at 1:22 AM Ajay Gupta wrote:
>
> Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller
> over I2C interface.
>
> This UCSI I2C driver uses I2C bus driver interface for communicating
> with Type-C controller.
>
FWIW,
Reviewed-by: Andy Shevchenko
for either version you choose (stack or heap).
> Signed-off-by: Ajay Gupta
> ---
> Changes from v1 -> v2
> Fixed identation in drivers/usb/typec/ucsi/Kconfig
> Changes from v2 -> v3
> Fixed most of comments from Heikki
> Rename ucsi_i2c_ccg.c -> ucsi_ccg.c
> Changes from v3 -> v4
> Fixed comments from Andy
> Changes from v4 -> v5
> Fixed comments from Andy
> Changes from v5 -> v6
> Fixed review comments from Greg
>
> drivers/usb/typec/ucsi/Kconfig| 10 +
> drivers/usb/typec/ucsi/Makefile | 2 +
> drivers/usb/typec/ucsi/ucsi_ccg.c | 382
> ++
> 3 files changed, 394 insertions(+)
> create mode 100644 drivers/usb/typec/ucsi/ucsi_ccg.c
>
> diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> index e36d6c7..7811888 100644
> --- a/drivers/usb/typec/ucsi/Kconfig
> +++ b/drivers/usb/typec/ucsi/Kconfig
> @@ -23,6 +23,16 @@ config TYPEC_UCSI
>
> if TYPEC_UCSI
>
> +config UCSI_CCG
> + tristate "UCSI Interface Driver for Cypress CCGx"
> + depends on I2C
> + help
> + This driver enables UCSI support on platforms that expose a
> + Cypress CCGx Type-C controller over I2C interface.
> +
> + To compile the driver as a module, choose M here: the module will be
> + called ucsi_ccg.
> +
> config UCSI_ACPI
> tristate "UCSI ACPI Interface Driver"
> depends on ACPI
> diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
> index 7afbea5..2f4900b 100644
> --- a/drivers/usb/typec/ucsi/Makefile
> +++ b/drivers/usb/typec/ucsi/Makefile
> @@ -8,3 +8,5 @@ typec_ucsi-y:= ucsi.o
> typec_ucsi-$(CONFIG_TRACING) += trace.o
>
> obj-$(CONFIG_UCSI_ACPI)+= ucsi_acpi.o
> +
> +obj-$(CONFIG_UCSI_CCG) += ucsi_ccg.o
> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
> b/drivers/usb/typec/ucsi/ucsi_ccg.c
> new file mode 100644
> index 000..65292bf
> --- /dev/null
> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> @@ -0,0 +1,382 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * UCSI driver for Cypress CCGx Type-C controller
> + *
> + * Copyright (C) 2017-2018 NVIDIA Corporation. All rights reserved.
> + * Author: Ajay Gupta
> + *
> + * Some code borrowed from drivers/usb/typec/ucsi/ucsi_acpi.c
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include "ucsi.h"
> +
> +struct ucsi_ccg {
> + struct device *dev;
> + struct ucsi *ucsi;
> + struct ucsi_ppm ppm;
> + struct i2c_client *client;
> + int irq;
> +};
> +
> +#define CCGX_I2C_RAB_DEVICE_MODE 0x00
> +#define CCGX_I2C_RAB_READ_SILICON_ID 0x2
> +#define CCGX_I2C_RAB_INTR_REG 0x06
> +#define CCGX_I2C_RAB_FW1_VERSION 0x28
> +#define CCGX_I2C_RAB_FW2_VERSION 0x20
> +#define CCGX_I2C_RAB_UCSI_CONTROL 0x39
> +#define CCGX_I2C_RAB_UCSI_CONTROL_STARTBIT(0)
> +#define CCGX_I2C_RAB_UCSI_CONTROL_STOP BIT(1)
> +#define CCGX_I2C_RAB_RESPONSE_REG 0x7E
> +#define CCGX_I2C_RAB_UCSI_DATA_BLOCK 0xf000
> +
> +static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
> +{
> + struct i2c_client *client = uc->client;
> + unsigned char *buf;
> + struct i2c_msg *msgs;
> + u32 rlen, rem_len = len;
> + int status;
> +
> + buf = kzalloc(2, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + msgs = kcalloc(2, sizeof(struct i2c_msg), GFP_KERNEL);
> + if (!msgs) {
> + kfree(buf);
> + return -ENOMEM;
> + }
> +
> + msgs[0].addr = client->addr;
> + msgs[0].len = 2;
> + msgs[0].buf = buf;
> + msgs[1].addr = client->addr;
> + msgs[1].flags = I2C_M_RD;
> +
> + while (rem_len > 0) {
> + msgs[1].buf = [len - rem_len];
> + rlen = min_t(u16, rem_len, 4);
> + msgs[1].len = rlen;
> + put_unaligned_le16(rab, buf);
> + status = i2c_transfer(client->a
Re: [PATCH v6 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
On Thu, Sep 6, 2018 at 1:23 AM Ajay Gupta wrote: > > Latest NVIDIA GPU card has USB Type-C interface. There is a > Type-C controller which can be accessed over I2C. > > This driver adds I2C bus driver to communicate with Type-C controller. > I2C client driver will be part of USB Type-C UCSI driver. > FWIW, Reviewed-by: Andy Shevchenko > Signed-off-by: Ajay Gupta > --- > Changes from v1 -> v2 > None > Changes from v2 -> v3 > Fixed review comments from Andy and Thierry > Rename i2c-gpu.c -> i2c-nvidia-gpu.c > Changes from v3 -> v4 > Fixed review comments from Andy > Changes from v4 -> v5 > Fixed review comments from Andy > Changes from v5 -> v6 > None > > Documentation/i2c/busses/i2c-nvidia-gpu | 18 ++ > MAINTAINERS | 7 + > drivers/i2c/busses/Kconfig | 9 + > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-nvidia-gpu.c | 405 > > 5 files changed, 440 insertions(+) > create mode 100644 Documentation/i2c/busses/i2c-nvidia-gpu > create mode 100644 drivers/i2c/busses/i2c-nvidia-gpu.c > > diff --git a/Documentation/i2c/busses/i2c-nvidia-gpu > b/Documentation/i2c/busses/i2c-nvidia-gpu > new file mode 100644 > index 000..31884d2 > --- /dev/null > +++ b/Documentation/i2c/busses/i2c-nvidia-gpu > @@ -0,0 +1,18 @@ > +Kernel driver i2c-nvidia-gpu > + > +Datasheet: not publicly available. > + > +Authors: > + Ajay Gupta > + > +Description > +--- > + > +i2c-nvidia-gpu is a driver for I2C controller included in NVIDIA Turing > +and later GPUs and it is used to communicate with Type-C controller on GPUs. > + > +If your 'lspci -v' listing shows something like the following, > + > +01:00.3 Serial bus controller [0c80]: NVIDIA Corporation Device 1ad9 (rev a1) > + > +then this driver should support the I2C controller of your GPU. > diff --git a/MAINTAINERS b/MAINTAINERS > index 9ad052a..2d1c5a1 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6797,6 +6797,13 @@ L: linux-a...@vger.kernel.org > S: Maintained > F: drivers/i2c/i2c-core-acpi.c > > +I2C CONTROLLER DRIVER FOR NVIDIA GPU > +M: Ajay Gupta > +L: linux-...@vger.kernel.org > +S: Maintained > +F: Documentation/i2c/busses/i2c-nvidia-gpu > +F: drivers/i2c/busses/i2c-nvidia-gpu.c > + > I2C MUXES > M: Peter Rosin > L: linux-...@vger.kernel.org > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 451d4ae..eed827b 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -224,6 +224,15 @@ config I2C_NFORCE2_S4985 > This driver can also be built as a module. If so, the module > will be called i2c-nforce2-s4985. > > +config I2C_NVIDIA_GPU > + tristate "NVIDIA GPU I2C controller" > + depends on PCI > + help > + If you say yes to this option, support will be included for the > + NVIDIA GPU I2C controller which is used to communicate with the > GPU's > + Type-C controller. This driver can also be built as a module called > + i2c-nvidia-gpu. > + > config I2C_SIS5595 > tristate "SiS 5595" > depends on PCI > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index 18b26af..d499813 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -140,5 +140,6 @@ obj-$(CONFIG_I2C_SIBYTE)+= i2c-sibyte.o > obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o > obj-$(CONFIG_SCx200_ACB) += scx200_acb.o > obj-$(CONFIG_I2C_FSI) += i2c-fsi.o > +obj-$(CONFIG_I2C_NVIDIA_GPU) += i2c-nvidia-gpu.o > > ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG > diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c > b/drivers/i2c/busses/i2c-nvidia-gpu.c > new file mode 100644 > index 000..2ce6706 > --- /dev/null > +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c > @@ -0,0 +1,405 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Nvidia GPU I2C controller Driver > + * > + * Copyright (C) 2018 NVIDIA Corporation. All rights reserved. > + * Author: Ajay Gupta > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +/* I2C definitions */ > +#define I2C_MST_CNTL 0x00 > +#define I2C_MST_CNTL_GEN_START BIT(0) > +#define I2C_MST_CNTL_GEN_STOP BIT(1) > +#define I2C_MST_CNTL_CMD_NONE (0 <<
Re: [PATCH v3 00/10] usb: typec: A few more improvements for Intel CHT
On Tue, Sep 4, 2018 at 2:22 PM Heikki Krogerus wrote: > > Hi, > > These patches will introduce a few improvements to the USB Type-C > support on Intel CHT platform. In this series I'm preparing Intel CHT > mux handling for DisplayPort Alt Mode support, but please note that, > after these patches the DisplayPort alternate mode will still not work > out of the box on CHT platform. Changes to the fusb302.c, and possibly > tcpm.c are still needed. > > This version will only fix the issue Hans pointed out in v2. Instead > of replacing the connection that assigned the mux to the USB Type-C > port with a connection that assigns the mux to the alternate mode > device, we keep all the existing connections and add a new one for the > alternate mode device. That way USB3 devices continue to be enumerated > as USB3 devices instead of USB2 devices. > > The origin thread can be read here: > https://www.spinics.net/lists/linux-usb/msg172033.html > > It seems you forgot my tag Acked-by: Andy Shevchenko for PDx86 bits on condition that Hans is okay with them. > Heikki Krogerus (10): > usb: typec: Take care of driver module reference counting > usb: roles: Handle driver reference counting > platform: x86: intel_cht_int33fe: Add dependency on muxes > drivers: base: Helpers for adding device connection descriptions > platform: x86: intel_cht_int33fe: Register all connections at once > platform: x86: intel_cht_int33fe: Add connection for the DP alt mode > platform: x86: intel_cht_int33fe: Add connections for the USB Type-C > port > usb: typec: class: Don't use port parent for getting mux handles > platform: x86: intel_cht_int33fe: Remove the old connections for the > muxes > usb: typec: fusb302: reorganizing the probe function a little > > drivers/platform/x86/Kconfig | 2 ++ > drivers/platform/x86/intel_cht_int33fe.c | 27 - > drivers/usb/common/roles.c | 15 -- > drivers/usb/typec/class.c| 38 ++-- > drivers/usb/typec/fusb302/fusb302.c | 25 ++-- > drivers/usb/typec/mux.c | 17 --- > include/linux/device.h | 24 +++ > 7 files changed, 87 insertions(+), 61 deletions(-) > > -- > 2.18.0 > -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 00/10] usb: typec: A few more improvements for Intel CHT
On Mon, Sep 3, 2018 at 4:37 PM Heikki Krogerus wrote: > > Hi, > > I've now removed the conditional creation of the mux device, and the > connection for it that was checked in intel_cht_int33fe.c. I'm instead > making the intel_cht_int33fe driver depend on the mux drivers. I also > added a trivial cleanup patch (patch 10/10) for the fusb302.c to this > series, and also a few fixes (1/10 and 2/10) to the mux handling. > > The origin thread can be read here: > https://www.spinics.net/lists/linux-usb/msg172033.html > > Acked-by: Andy Shevchenko for PDx86 bits on condition that Hans is fine with them. > Heikki Krogerus (10): > usb: typec: Take care of driver module reference counting > usb: roles: Handle driver reference counting > platform: x86: intel_cht_int33fe: Add dependency on muxes > drivers: base: Helpers for adding device connection descriptions > platform: x86: intel_cht_int33fe: Register all connections at once > platform: x86: intel_cht_int33fe: Fix the identifier for the mux > connection > platform: x86: intel_cht_int33fe: Add connections for the USB Type-C > port > usb: typec: class: Don't use port parent for getting mux handles > platform: x86: intel_cht_int33fe: Remove the old connections for the > muxes > usb: typec: fusb302: reorganizing the probe function a little > > drivers/platform/x86/Kconfig | 2 ++ > drivers/platform/x86/intel_cht_int33fe.c | 20 + > drivers/usb/common/roles.c | 15 -- > drivers/usb/typec/class.c| 38 ++-- > drivers/usb/typec/fusb302/fusb302.c | 25 ++-- > drivers/usb/typec/mux.c | 17 --- > include/linux/device.h | 24 +++ > 7 files changed, 82 insertions(+), 59 deletions(-) > > -- > 2.18.0 > -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/8] usb: typec: A few more improvements for Intel CHT
On Mon, Sep 3, 2018 at 10:19 AM Heikki Krogerus wrote: > > On Mon, Sep 03, 2018 at 09:17:13AM +0300, Andy Shevchenko wrote: > > On Fri, Aug 31, 2018 at 5:21 PM Heikki Krogerus > > wrote: > > > > > > Hi, > > > > > > The second last patch in this series will make it possible to use > > > multiport USB Type-C and PD controllers with the muxes. The CHT > > > connections are simply adapted to that. The rest of the series will > > > mainly allow us to use the USB Type-C on CHT boards even without a > > > USB device controller. > > > > > > > Throught which tree you are planning to direct this? > > The series is about USB Type-C, so USB would make sense to me. But > it's up to you guys of course. My vacation is about to start, and from my prospective patch series looks good (taking into account the changes Hans asked for). Thus, Acked-by: Andy Shevchenko for PDx86 bits on condition that Hans is fine with them. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/8] usb: typec: A few more improvements for Intel CHT
On Fri, Aug 31, 2018 at 5:21 PM Heikki Krogerus wrote: > > Hi, > > The second last patch in this series will make it possible to use > multiport USB Type-C and PD controllers with the muxes. The CHT > connections are simply adapted to that. The rest of the series will > mainly allow us to use the USB Type-C on CHT boards even without a > USB device controller. > Throught which tree you are planning to direct this? > > Heikki Krogerus (8): > drivers: base: Helpers for adding device connection descriptions > plarform: x86: intel_cht_int33fe: Register all connections at once > plarform: x86: intel_cht_int33fe: Use the USB role switch > conditionally > usb: xhci: pci: Only create Intel mux device when it's needed > plarform: x86: intel_cht_int33fe: Fix the identifier for the mux > connection > plarform: x86: intel_cht_int33fe: Add connections for the USB Type-C > port > usb: typec: class: Don't use port parent for getting mux handles > plarform: x86: intel_cht_int33fe: Remove the old connections for the > muxes > > drivers/platform/x86/intel_cht_int33fe.c | 34 +++-- > drivers/usb/host/xhci-pci.c | 20 +++-- > drivers/usb/typec/class.c| 38 ++-- > include/linux/device.h | 24 +++ > 4 files changed, 74 insertions(+), 42 deletions(-) > > -- > 2.18.0 > -- With Best Regards, Andy Shevchenko
Re: [PATCH 4/8] usb: xhci: pci: Only create Intel mux device when it's needed
On Fri, Aug 31, 2018 at 5:21 PM Heikki Krogerus
wrote:
>
> Only create thre Intel role mux device if the platform has
> USB peripheral controller PCI device.
>
> While here, enable the role mux on Apollo Lake platforms.
> +static int xhci_pci_board_has_udc(void)
> +{
> + struct pci_dev *udc = pci_get_class(PCI_CLASS_SERIAL_USB_DEVICE,
> NULL);
> +
> + if (udc) {
> + pci_dev_put(udc);
> + return true;
> + }
> + return false;
> +}
Looks like a code duplication with patch 3. Does it make sense to have
this in some header (usb.h?)?
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v4 2/2] usb: typec: ucsi: add support for Cypress CCGx
On Fri, Aug 31, 2018 at 12:41 AM Ajay Gupta wrote:
> Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller
> over I2C interface.
>
> This UCSI I2C driver uses I2C bus driver interface for communicating
> with Type-C controller.
> Changes from v3 -> v4
> Fixed comments from Andy
Unfortunatelly not all comments was addressed, see below.
> + if (err == ARRAY_SIZE(msgs)) {
> + err = 0;
> + } else if (err >= 0) {
> + dev_err(dev, "i2c_transfer failed, err %d\n", err);
> + return -EIO;
> + }
Shouldn't be simple
if (err < 0) {
...
return err;
}
?
> + if (err == ARRAY_SIZE(msgs)) {
> + err = 0;
> + } else if (err >= 0) {
> + dev_err(dev, "i2c_transfer failed, err %d\n", err);
> + return -EIO;
> + }
Ditto.
> + struct device *dev = uc->dev;
> + u8 data[64];
> + int err;
> + unsigned int count = 10;
Move this line upper a bit.
> + unsigned char buf[3] = {
> + CCGX_I2C_RAB_INTR_REG & 0xff, CCGX_I2C_RAB_INTR_REG >> 8,
> 0x0};
This should follow the style you applied earlier in the same file.
Also, & 0xff is redundant (better just to use >> 0).
> + struct ucsi_ccg *uc = container_of(ppm,
> + struct ucsi_ccg, ppm);
One line?
> +static int ucsi_ccg_cmd(struct ucsi_ppm *ppm, struct ucsi_control *ctrl)
> +{
> + struct ucsi_ccg *uc = container_of(ppm, struct ucsi_ccg, ppm);
> + int err = 0;
Redundant assignment.
> +
> + ppm->data->ctrl.raw_cmd = ctrl->raw_cmd;
> + err = ucsi_ccg_send_data(uc);
> +
> + return err;
> +}
> +static int ucsi_ccg_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
One line?
> +static const struct i2c_device_id ucsi_ccg_device_id[] = {
> + {"ccgx-ucsi", 0},
> + {},
Terminator better w/o comma.
> +};
> +MODULE_DEVICE_TABLE(i2c, ucsi_ccg_device_id);
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v4 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
On Fri, Aug 31, 2018 at 12:41 AM Ajay Gupta wrote:
>
> Latest NVIDIA GPU card has USB Type-C interface. There is a
> Type-C controller which can be accessed over I2C.
>
> This driver adds I2C bus driver to communicate with Type-C controller.
> I2C client driver will be part of USB Type-C UCSI driver.
Thanks for an update, my comments below.
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
+ blank line.
> +#include
> +struct gpu_i2c_dev {
> + struct device *dev;
> + void __iomem *regs;
> + struct i2c_adapter adapter;
> + struct i2c_client *client;
> + struct mutex mutex; /* to sync read/write */
> + bool do_start;
> +};
> +static int i2c_check_status(struct gpu_i2c_dev *i2cd)
> +{
> + unsigned long target = jiffies + msecs_to_jiffies(1000);
> + u32 val;
> +
> + do {
> + val = readl(i2cd->regs + I2C_MST_CNTL);
> + if ((val & I2C_MST_CNTL_CYCLE_TRIGGER) !=
> + I2C_MST_CNTL_CYCLE_TRIGGER)
Part after != redundant since it's one bit.
But I'm fine with current as well.
> + break;
> + if ((val & I2C_MST_CNTL_STATUS) !=
> + I2C_MST_CNTL_STATUS_BUS_BUSY)
> + break;
> + usleep_range(1000, 2000);
> + } while (time_is_after_jiffies(target));
> +
Redundant.
> + if (time_is_before_jiffies(target))
> + return -EIO;
> +
> + val = readl(i2cd->regs + I2C_MST_CNTL);
> + switch (val & I2C_MST_CNTL_STATUS) {
> + case I2C_MST_CNTL_STATUS_OKAY:
> + return 0;
> + case I2C_MST_CNTL_STATUS_NO_ACK:
> + return -EIO;
> + case I2C_MST_CNTL_STATUS_TIMEOUT:
> + return -ETIME;
> + case I2C_MST_CNTL_STATUS_BUS_BUSY:
> + return -EBUSY;
> + default:
> + break;
return 0; ?
> + }
> + return 0;
See above.
> +}
> +static int i2c_write(struct gpu_i2c_dev *i2cd, u8 data)
> +{
> + u32 val;
> +
> + writel(data, i2cd->regs + I2C_MST_DATA);
> +
> + val = I2C_MST_CNTL_CMD_WRITE | (1 << I2C_MST_CNTL_BURST_SIZE_SHIFT) |
> + I2C_MST_CNTL_GEN_NACK;
> + val &= ~(I2C_MST_CNTL_GEN_START | I2C_MST_CNTL_GEN_STOP
> + | I2C_MST_CNTL_GEN_RAB);
"|" should be on previous line to follow common style in this module.
> + writel(val, i2cd->regs + I2C_MST_CNTL);
> +
> + return i2c_check_status(i2cd);
> +}
> +
> +static int gpu_i2c_master_xfer(struct i2c_adapter *adap,
> + struct i2c_msg *msgs, int num)
> +{
> + struct gpu_i2c_dev *i2cd = i2c_get_adapdata(adap);
> + struct device *dev = i2cd->dev;
> + int status;
> + int i, j;
> +stop:
> + status = i2c_stop(i2cd);
> + if (status < 0)
> + dev_err(dev, "i2c_stop error %x", status);
> +unlock:
> + mutex_unlock(>mutex);
> + return i;
Shouldn't it return status in case of error?
> +}
> +/*
> + * This driver is for the Nvidia GPU cards with USB Type-C interface.
> + * We want to identify the cards using vendor ID and class code.
> + * Currently there is no class code defined for UCSI device over PCI
> + * so using UNKNOWN.
> + */
So, I didn't see how it *guarantees* no collision with other devices
of the same class...
> +#define PCI_CLASS_SERIAL_UNKNOWN 0x0c80
> +static const struct pci_device_id gpu_i2c_ids[] = {
> + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> + PCI_CLASS_SERIAL_UNKNOWN << 8, 0xff00},
...for now, I would suggest to be more stricted, i.e.
{ PCI_VDEVICE(NVIDIA, 0x1ad9) },
Whenever the class appears it can be added later on.
> + { }
> +};
> +MODULE_DEVICE_TABLE(pci, gpu_i2c_ids);
> +
> +static int gpu_i2c_probe(struct pci_dev *dev, const struct pci_device_id *id)
Use pdev instead of dev to distinguish struct device from struct
pci_dev type in variable name.
> +static void gpu_i2c_remove(struct pci_dev *dev)
Ditto.
> + struct gpu_i2c_dev *i2cd = pci_get_drvdata(to_pci_dev(dev));
Isn't the same as dev_get_drvdata() ?
> + struct gpu_i2c_dev *i2cd = pci_get_drvdata(to_pci_dev(dev));
Ditto.
--
With Best Regards,
Andy Shevchenko
[PATCH v1] USB: wusbcore: Switch to bitmap_zalloc()
Switch to bitmap_zalloc() to show clearly what we are allocating.
Besides that it returns pointer of bitmap type instead of opaque void *.
Signed-off-by: Andy Shevchenko
---
drivers/usb/wusbcore/wa-rpipe.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/wusbcore/wa-rpipe.c b/drivers/usb/wusbcore/wa-rpipe.c
index 38884aac862b..a5734cbcd5ad 100644
--- a/drivers/usb/wusbcore/wa-rpipe.c
+++ b/drivers/usb/wusbcore/wa-rpipe.c
@@ -470,9 +470,7 @@ int rpipe_get_by_ep(struct wahc *wa, struct
usb_host_endpoint *ep,
int wa_rpipes_create(struct wahc *wa)
{
wa->rpipes = le16_to_cpu(wa->wa_descr->wNumRPipes);
- wa->rpipe_bm = kcalloc(BITS_TO_LONGS(wa->rpipes),
- sizeof(unsigned long),
- GFP_KERNEL);
+ wa->rpipe_bm = bitmap_zalloc(wa->rpipes, GFP_KERNEL);
if (wa->rpipe_bm == NULL)
return -ENOMEM;
return 0;
@@ -487,7 +485,7 @@ void wa_rpipes_destroy(struct wahc *wa)
dev_err(dev, "BUG: pipes not released on exit: %*pb\n",
wa->rpipes, wa->rpipe_bm);
}
- kfree(wa->rpipe_bm);
+ bitmap_free(wa->rpipe_bm);
}
/*
--
2.18.0
Re: [PATCH v3 2/2] usb: typec: ucsi: add support for Cypress CCGx
lt; 0) {
> + dev_err(uc->dev, "ccg_write failed, err %d\n", err);
> + return -EIO;
> + }
> +
> + err = ccg_write(uc, *(u16 *)(buf + 2), buf2, sizeof(buf2));
> + if (err < 0) {
> + dev_err(uc->dev, "ccg_write failed, err %d\n", err);
> + return -EIO;
> + }
> +
> + return err;
> +}
> +
> +static int ucsi_ccg_recv_data(struct ucsi_ccg *uc)
> +{
> + static int first = 1;
> + int err;
> + unsigned char buf[6] = {
> + 0x0, (CCGX_I2C_RAB_UCSI_DATA_BLOCK >> 8),
> + 0x4, (CCGX_I2C_RAB_UCSI_DATA_BLOCK >> 8),
> + 0x10, (CCGX_I2C_RAB_UCSI_DATA_BLOCK >> 8),
Redundant parens.
> + };
> + u8 *ppm = (u8 *)uc->ppm.data;
> +
> + if (first) {
> + err = ccg_read(uc, *(u16 *)buf, ppm, 0x2);
> + if (err < 0) {
> + dev_err(uc->dev, "ccg_read failed, err %d\n", err);
> + return -EIO;
> + }
> + first = 1;
> + }
> +
> + err = ccg_read(uc, *(u16 *)(buf + 2), ppm + 0x4, 0x4);
> + if (err < 0) {
> + dev_err(uc->dev, "ccg_read failed, err %d\n", err);
> + return -EIO;
> + }
> +
> + err = ccg_read(uc, *(u16 *)(buf + 4), ppm + 0x10, 0x10);
> + if (err < 0) {
> + dev_err(uc->dev, "ccg_read failed, err %d\n", err);
> + return -EIO;
> + }
> +
> + return err;
> +}
> +
> +static int ucsi_ccg_ack_interrupt(struct ucsi_ccg *uc)
> +{
> + int err;
> + unsigned char buf[3] = {(CCGX_I2C_RAB_INTR_REG & 0xff),
> + (CCGX_I2C_RAB_INTR_REG >> 8), 0x0};
Different style, redundant parens.
> +
> + err = ccg_read(uc, *(u16 *)buf, [2], 0x1);
> + if (err < 0) {
> + dev_err(uc->dev, "ccg_read failed, err %d\n", err);
> + return -EIO;
> + }
> +
> + err = ccg_write(uc, *(u16 *)buf, [2], 0x1);
> + if (err < 0) {
> + dev_err(uc->dev, "ccg_read failed, err %d\n", err);
> + return -EIO;
> + }
> +
> + return err;
> +}
> +
> +static int ucsi_ccg_sync(struct ucsi_ppm *ppm)
> +{
> + struct ucsi_ccg *uc = container_of(ppm,
> + struct ucsi_ccg, ppm);
> + int err;
> +
> + err = ucsi_ccg_recv_data(uc);
> + if (err < 0) {
> + dev_err(uc->dev, "ucsi_ccg_recv_data() err %d\n", err);
> + return 0;
> + }
> +
> + /* ack interrupt to allow next command to run */
> + err = ucsi_ccg_ack_interrupt(uc);
> + if (err < 0)
> + dev_err(uc->dev, "ucsi_ccg_ack_interrupt() err %d\n", err);
> +
> + return 0;
> +}
> +
> +static int ucsi_ccg_cmd(struct ucsi_ppm *ppm, struct ucsi_control *ctrl)
> +{
> + struct ucsi_ccg *uc = container_of(ppm,
> + struct ucsi_ccg, ppm);
Why not one line?
> + int err = 0;
> +
> + ppm->data->ctrl.raw_cmd = ctrl->raw_cmd;
> + err = ucsi_ccg_send_data(uc);
> +
> + return err;
> +}
> +
> +static irqreturn_t ccg_irq_handler(int irq, void *data)
> +{
> + struct ucsi_ccg *uc = data;
> +
> + ucsi_notify(uc->ucsi);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int ucsi_ccg_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
One line?
> +{
> + struct device *dev = >dev;
> + struct ucsi_ccg *uc;
> + int status;
> +
> + uc = devm_kzalloc(dev, sizeof(*uc), GFP_KERNEL);
> + if (!uc)
> + return -ENOMEM;
> +
> + uc->ppm.data = devm_kzalloc(dev, sizeof(struct ucsi_data),
> GFP_KERNEL);
> + if (!uc->ppm.data)
> + return -ENOMEM;
> +
> + uc->ppm.cmd = ucsi_ccg_cmd;
> + uc->ppm.sync = ucsi_ccg_sync;
> + uc->dev = dev;
> + uc->client = client;
> +
> + /* reset ccg device and initialize ucsi */
> + status = ucsi_ccg_init(uc);
> + if (status < 0) {
> + dev_err(uc->dev, "ucsi_ccg_init failed - %d\n", status);
> + return status;
> + }
> +
> + uc->irq = client->irq;
> +
> + status = devm_request_threaded_irq(dev, uc->irq, NULL,
> ccg_irq_handler,
> + IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
ONESHORT can be dropped here (it's set based on condition when irq
handler NULL and threadirq is not).
> + dev_name(dev), uc);
> + if (status < 0) {
> + dev_err(uc->dev, "request_threaded_irq failed - %d\n",
> status);
> + return status;
> + }
> +
> + uc->ucsi = ucsi_register_ppm(dev, >ppm);
> + if (IS_ERR(uc->ucsi)) {
> + dev_err(uc->dev, "ucsi_register_ppm failed\n");
> + return PTR_ERR(uc->ucsi);
> + }
> +
> + i2c_set_clientdata(client, uc);
> + return 0;
> +}
> +
> +static int ucsi_ccg_remove(struct i2c_client *client)
> +{
> + struct ucsi_ccg *uc = i2c_get_clientdata(client);
> +
> + ucsi_unregister_ppm(uc->ucsi);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id ucsi_ccg_device_id[] = {
> + {"ccgx-ucsi", 0},
> + {},
Terminator line better without comma.
> +};
> +MODULE_DEVICE_TABLE(i2c, ucsi_ccg_device_id);
> +
> +static struct i2c_driver ucsi_ccg_driver = {
> + .driver = {
> + .name = "ucsi_ccg",
> + },
> + .probe = ucsi_ccg_probe,
> + .remove = ucsi_ccg_remove,
> + .id_table = ucsi_ccg_device_id,
> +};
> +
> +module_i2c_driver(ucsi_ccg_driver);
> +
> +MODULE_AUTHOR("Ajay Gupta ");
> +MODULE_DESCRIPTION("UCSI driver for Cypress CCGx Type-C controller");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v3 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
On Thu, Aug 30, 2018 at 3:35 AM wrote: > > From: Ajay Gupta > > Latest NVIDIA GPU card has USB Type-C interface. There is a > Type-C controller which can be accessed over I2C. > > This driver adds I2C bus driver to communicate with Type-C controller. > I2C client driver will be part of USB Type-C UCSI driver. > Some small comments below, after addressing them Reviewed-by: Andy Shevchenko > Signed-off-by: Ajay Gupta > --- > Changes from v1 -> v2 > None > Changes from v2 -> v3 > Fixed review comments from Andy and Thierry > Rename i2c-gpu.c -> i2c-nvidia-gpu.c > > Documentation/i2c/busses/i2c-nvidia-gpu | 18 ++ > MAINTAINERS | 7 + > drivers/i2c/busses/Kconfig | 9 + > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-nvidia-gpu.c | 389 > > 5 files changed, 424 insertions(+) > create mode 100644 Documentation/i2c/busses/i2c-nvidia-gpu > create mode 100644 drivers/i2c/busses/i2c-nvidia-gpu.c > > diff --git a/Documentation/i2c/busses/i2c-nvidia-gpu > b/Documentation/i2c/busses/i2c-nvidia-gpu > new file mode 100644 > index 000..31884d2 > --- /dev/null > +++ b/Documentation/i2c/busses/i2c-nvidia-gpu > @@ -0,0 +1,18 @@ > +Kernel driver i2c-nvidia-gpu > + > +Datasheet: not publicly available. > + > +Authors: > + Ajay Gupta > + > +Description > +--- > + > +i2c-nvidia-gpu is a driver for I2C controller included in NVIDIA Turing > +and later GPUs and it is used to communicate with Type-C controller on GPUs. > + > +If your 'lspci -v' listing shows something like the following, > + > +01:00.3 Serial bus controller [0c80]: NVIDIA Corporation Device 1ad9 (rev a1) > + > +then this driver should support the I2C controller of your GPU. > diff --git a/MAINTAINERS b/MAINTAINERS > index 9ad052a..2d1c5a1 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6797,6 +6797,13 @@ L: linux-a...@vger.kernel.org > S: Maintained > F: drivers/i2c/i2c-core-acpi.c > > +I2C CONTROLLER DRIVER FOR NVIDIA GPU > +M: Ajay Gupta > +L: linux-...@vger.kernel.org > +S: Maintained > +F: Documentation/i2c/busses/i2c-nvidia-gpu > +F: drivers/i2c/busses/i2c-nvidia-gpu.c > + > I2C MUXES > M: Peter Rosin > L: linux-...@vger.kernel.org > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 451d4ae..eed827b 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -224,6 +224,15 @@ config I2C_NFORCE2_S4985 > This driver can also be built as a module. If so, the module > will be called i2c-nforce2-s4985. > > +config I2C_NVIDIA_GPU > + tristate "NVIDIA GPU I2C controller" > + depends on PCI > + help > + If you say yes to this option, support will be included for the > + NVIDIA GPU I2C controller which is used to communicate with the > GPU's > + Type-C controller. This driver can also be built as a module called > + i2c-nvidia-gpu. > + > config I2C_SIS5595 > tristate "SiS 5595" > depends on PCI > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index 18b26af..d499813 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -140,5 +140,6 @@ obj-$(CONFIG_I2C_SIBYTE)+= i2c-sibyte.o > obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o > obj-$(CONFIG_SCx200_ACB) += scx200_acb.o > obj-$(CONFIG_I2C_FSI) += i2c-fsi.o > +obj-$(CONFIG_I2C_NVIDIA_GPU) += i2c-nvidia-gpu.o > > ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG > diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c > b/drivers/i2c/busses/i2c-nvidia-gpu.c > new file mode 100644 > index 000..fa01187 > --- /dev/null > +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c > @@ -0,0 +1,389 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Nvidia GPU I2C controller Driver > + * > + * Copyright (C) 2018 NVIDIA Corporation. All rights reserved. > + * Author: Ajay Gupta > + * Redundant line. > + */ > +#include This should go after linux/* ones... > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + ...here. > +/* I2C definitions */ > +#define I2C_MST_CNTL 0x0 > +#define I2C_MST_CNTL_GEN_START BIT(0) > +#define I2C_MST_CNTL_GEN_STOP BIT(1) > +#define I2C_MST_CNTL_CMD_NONE (0 << 2) > +#define I2C_MST_CNTL_CMD_READ (1 << 2
[PATCH v1] usb: dwc3: core: Clean up ULPI device
If dwc3_core_init_mode() fails with deferred probe, next probe fails on sysfs with sysfs: cannot create duplicate filename '/devices/pci:00/:00:11.0/dwc3.0.auto/dwc3.0.auto.ulpi' To avoid this failure, clean up ULPI device. Signed-off-by: Andy Shevchenko --- drivers/usb/dwc3/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index fdf80482f4cf..923320d37c86 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -1503,6 +1503,7 @@ static int dwc3_probe(struct platform_device *pdev) err5: dwc3_event_buffers_cleanup(dwc); + dwc3_ulpi_exit(dwc); err4: dwc3_free_scratch_buffers(dwc); -- 2.18.0
Re: [PATCH v2 1/7] usb: chipidea: Add dynamic pinctrl selection
On Mon, Aug 27, 2018 at 12:33 PM Loic Poulain wrote: > > Some hardware implementations require to configure pins differently > according to the USB role (host/device), this can be an update of the > pins routing or a simple GPIO value change. > > This patch introduces new optional "host" and "device" pinctrls. > If these pinctrls are defined by the device, they are respectively > selected on host/device role start. > > If a default pinctrl exist, it is restored on host/device role stop. > v2: includes ordering I didn't see a change here. Anyway, it's a minor. Free to fix whenever more serious stuff appears. -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 2/2] usb: typec: ucsi: add support for Cypress CCGx
> --- > This email message is for the sole use of the intended recipient(s) and may > contain > confidential information. Any unauthorized review, use, disclosure or > distribution > is prohibited. If you are not the intended recipient, please contact the > sender by > reply email and destroy all copies of the original message. > --- You have an issue here. -- With Best Regards, Andy Shevchenko
Re: [PATCH 1/6] usb: chipidea: Add dynamic pinctrl selection
On Tue, Aug 21, 2018 at 4:57 PM Loic Poulain wrote: > > Some hardware implementations require to configure pins differently > according to the USB role (host/device), this can be an update of the > pins routing or a simple GPIO value change. > > This patch introduces new optional "host" and "device" pinctrls. > If these pinctrls are defined by the device, they are respectively > selected on host/device role start. > > If a default pinctrl exist, it is restored on host/device role stop. > #include > #include > #include > +#include A nit: Even in this context it would be nice to keep *some* order. > #include > #include > #include > + p = pinctrl_lookup_state(platdata->pctl, "default"); > + p = pinctrl_lookup_state(platdata->pctl, "host"); > + p = pinctrl_lookup_state(platdata->pctl, "device"); I'm wondering if we have any rules applied to these names. > #include > #include > #include > +#include Ditto about ordering. > + if (ci->platdata->pins_host) > + pinctrl_select_state(ci->platdata->pctl, > +ci->platdata->pins_host); Hmm... Do we need to have a condition above? > + if (ci->platdata->pins_host && ci->platdata->pins_default) > + pinctrl_select_state(ci->platdata->pctl, > +ci->platdata->pins_default); Ditto about conditional. > #include > #include > #include > +#include Ditto about ordering. > + if (ci->platdata->pins_device) > + pinctrl_select_state(ci->platdata->pctl, > +ci->platdata->pins_device); > + if (ci->platdata->pins_device && ci->platdata->pins_default) > + pinctrl_select_state(ci->platdata->pctl, > +ci->platdata->pins_default); Ditto about conditional. -- With Best Regards, Andy Shevchenko
Re: [PATCH 3/4] usb: dwc3: trace: log ep commands in hex
On Mon, Aug 20, 2018 at 3:21 PM Laurent Pinchart
wrote:
>
> Hi Andy,
>
> On Monday, 20 August 2018 15:06:31 EEST Andy Shevchenko wrote:
> > On Mon, Aug 20, 2018 at 2:25 PM, Laurent Pinchart wrote:
> > >> - TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x --> status: %s",
> > >> + TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status: %s",
> > >
> > > How about 0x%x ?
> >
> > Side note: # is one character less for the same.
>
> Doesn't that print 0 instead of 0x0 ? There's no ambiguity with 0, but I find
> that always printing the 0x is more consistent. I'll leave that up to Felipe,
> I'm OK with both options.
# The value should be converted to an "alternate form".
For o conversions, the first character of
the output string is made zero (by prefixing a 0 if it
was not zero already). For x and X con‐
versions, a nonzero result has the string "0x" (or "0X"
for X conversions) prepended to it.
--
With Best Regards,
Andy Shevchenko
Re: [PATCH 3/4] usb: dwc3: trace: log ep commands in hex
On Mon, Aug 20, 2018 at 2:25 PM, Laurent Pinchart
wrote:
>> - TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x --> status: %s",
>> + TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status: %s",
>
> How about 0x%x ?
Side note: # is one character less for the same.
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v2] usb: musb: gadget: Fix big-endian arch issue
On Fri, Aug 3, 2018 at 12:03 PM, Alexey Spirkov wrote: > Existing code is not applicable to big-endian machines > ctrlrequest fields received in USB endian - i.e. in little-endian > and should be converted to cpu endianness before usage. > - epnum = (u8) ctrlrequest->wIndex; > + epnum = (u8) le16_to_cpu(ctrlrequest->wIndex); > + u16 reqval = le16_to_cpu(ctrlrequest->wValue); > + u16 reqidx = le16_to_cpu(ctrlrequest->wIndex); I'm wondering, if you run make with C=1 CF=-D__CHECK_ENDIAN__ before and after your change. -- With Best Regards, Andy Shevchenko -- 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: serial: ftdi_sio: Add support for CBUS GPIO
On Wed, Aug 1, 2018 at 6:46 PM, Loic Poulain wrote:
> Some FTDI devices like FTX or FT232R support CBUS Bit Bang mode on CBUS
> pins, allowing host to control them via simple USB control transfers.
> To make use of a CBUS pin in Bit Bang mode, the pin must be configured
> to I/O mode in the FTDI EEPROM. This mode perfectly coexists with regular
> USB to Serial function.
>
> In this implementation, a GPIO controller is registered on FTDI probe
> if at least one CBUS pin is configured for I/O mode. For now, only
> FTX devices are supported.
>
> This patch is based on previous Stefan Agner implementation tentative
> on LKML ([PATCH 0/2] FTDI CBUS GPIO support).
The best approach to refer to a mail is to put a message-id, something like
(... [1].)
[1]: Message-Id: <...message-id...>
> +static int ftdi_read_eeprom(struct usb_device *udev, unsigned int off,
> + void *val, size_t bytes)
> +{
> + if (bytes % 2) /* 16-bit eeprom */
> + return -EINVAL;
Why is it fatal?
You may read one byte less (and provide an error code like -EIO, or
whatever fits better) or more (and provide exact amount asked).
> + return 0;
> +}
> + rv = ftdi_read_pins(fgc->port, );
> + if (rv)
> + dev_err(>dev, "Unable to read CBUS GPIO pins, %d\n",
> rv);
Why not to return an error?
(Message itself sounds like a noise. Perhaps, dev_dbg() or throw away.
> + rv = ftdi_set_bitmode(fgc->port, fgc->cbus_mask,
> FTDI_SIO_BITMODE_CBUS);
> + if (rv)
> + dev_err(>dev, "Unable set CBUS Bit-Bang mode, %d\n",
> rv);
Ditto WRT message.
> +static int ftdi_register_cbus_gpiochip(struct usb_serial_port *port)
> +{
> + struct ftdi_private *priv = usb_get_serial_port_data(port);
> + struct usb_device *udev = port->serial->dev;
> + struct ftdi_gpiochip *fgc;
> + int rv;
> +
> + fgc = kzalloc(sizeof(*fgc), GFP_KERNEL);
> + if (!fgc)
> + return -ENOMEM;
devm_ ?
> + cbus_mux = kmalloc(4, GFP_KERNEL);
> + if (!cbus_mux)
> + return -ENOMEM;
Is it mandatory to alloc so small amount on heap in this case?
> + /* CBUS pins are individually configurable via 8-bit MUX
> control
> +* value, living at 0x1a for CBUS0. cf application note
> AN_201.
> +*/
Is it a comment style used in this file?
> + rv = ftdi_read_eeprom(udev, 0x1a, cbus_mux, 4);
0x1a is a magic.
> + rv = gpiochip_add_data(>gc, fgc);
> + if (rv) {
> + dev_err(>dev, "Unable to add gpiochip\n");
> + kfree(fgc);
> + return rv;
> + }
devm_ ?
> + return 0;
> +}
> +
> +static void ftdi_unregister_cbus_gpiochip(struct usb_serial_port *port)
> +{
> + struct ftdi_private *priv = usb_get_serial_port_data(port);
> + struct ftdi_gpiochip *fgc = priv->fgc;
> +
> + if (fgc) {
How you can be here when fgc == NULL?!
> + gpiochip_remove(>gc);
> + kfree(fgc);
> + }
> +}
I guess complete function will go away when you switch to devm.
--
With Best Regards,
Andy Shevchenko
--
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: dwc3: Set default mode for dwc_usb31
On Fri, Jul 27, 2018 at 1:14 PM, Felipe Balbi wrote:
>>>>>> +
>>>>>> + /*
>>>>>> +* dwc_usb31 does not support OTG mode. If the controller
>>>>>> +* supports DRD but the dr_mode is not specified or set
>>>>>> to OTG,
>>>>>> +* then set the mode to peripheral.
>>>>>> +*/
>>>>>> + if (mode == USB_DR_MODE_OTG && dwc3_is_usb31(dwc))
>>>>> shouldn't be simple
>>>>>
>>>>> else if (dwc3_is_usb31(...))
>>>>>
>>>>> ?
>>>
>>> Actually, no. We want to set the mode to peripheral _only_ when dr_mode
>>> was not specified or set to OTG. Just checking for dwc3_is_usb31(...) is
>>> not enough.
>>
>> How come?
>>
>> If I read the code correctly...
>>
>> When you go to default case in this switch it's possible if and only
>> if you have mode _exactly_ OTG. You can't have mode unknown here
>> either.
>> The check is redundant and absence of else adds additional burden on
>> the all the rest cases.
>
> Look a little closer
>
>> mode = dwc->dr_mode;
>> hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>>
>> switch (hw_mode) {
> ^^^
> Switching on hw_mode, not mode.
Ah, indeed. That's what I have missed.
Thanks for detailed explanation!
--
With Best Regards,
Andy Shevchenko
--
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: dwc3: Set default mode for dwc_usb31
On Fri, Jul 27, 2018 at 2:02 AM, Thinh Nguyen wrote: > On 7/26/2018 2:59 PM, Thinh Nguyen wrote: >> On 7/26/2018 2:32 PM, Andy Shevchenko wrote: >>> On Thu, Jul 26, 2018 at 11:52 PM, Thinh Nguyen >>> wrote: >>>> dwc_usb31 does not support OTG mode. If the controller supports DRD but >>>> the dr_mode is not specified or set to OTG, then set the mode to >>>> peripheral. >>>> >>>> Signed-off-by: Thinh Nguyen >>>> --- >>>> drivers/usb/dwc3/core.c | 8 >>>> 1 file changed, 8 insertions(+) >>>> >>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>> index 21e4931d0cc0..64ba664d467c 100644 >>>> --- a/drivers/usb/dwc3/core.c >>>> +++ b/drivers/usb/dwc3/core.c >>>> @@ -78,6 +78,14 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc) >>>> mode = USB_DR_MODE_HOST; >>>> else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET)) >>>> mode = USB_DR_MODE_PERIPHERAL; >>>> + >>>> + /* >>>> +* dwc_usb31 does not support OTG mode. If the controller >>>> +* supports DRD but the dr_mode is not specified or set to >>>> OTG, >>>> +* then set the mode to peripheral. >>>> +*/ >>>> + if (mode == USB_DR_MODE_OTG && dwc3_is_usb31(dwc)) >>> shouldn't be simple >>> >>> else if (dwc3_is_usb31(...)) >>> >>> ? > > Actually, no. We want to set the mode to peripheral _only_ when dr_mode > was not specified or set to OTG. Just checking for dwc3_is_usb31(...) is > not enough. How come? If I read the code correctly... When you go to default case in this switch it's possible if and only if you have mode _exactly_ OTG. You can't have mode unknown here either. The check is redundant and absence of else adds additional burden on the all the rest cases. -- With Best Regards, Andy Shevchenko -- 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: dwc3: Set default mode for dwc_usb31
On Thu, Jul 26, 2018 at 11:52 PM, Thinh Nguyen
wrote:
> dwc_usb31 does not support OTG mode. If the controller supports DRD but
> the dr_mode is not specified or set to OTG, then set the mode to
> peripheral.
>
> Signed-off-by: Thinh Nguyen
> ---
> drivers/usb/dwc3/core.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 21e4931d0cc0..64ba664d467c 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -78,6 +78,14 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc)
> mode = USB_DR_MODE_HOST;
> else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET))
> mode = USB_DR_MODE_PERIPHERAL;
> +
> + /*
> +* dwc_usb31 does not support OTG mode. If the controller
> +* supports DRD but the dr_mode is not specified or set to
> OTG,
> +* then set the mode to peripheral.
> +*/
> + if (mode == USB_DR_MODE_OTG && dwc3_is_usb31(dwc))
shouldn't be simple
else if (dwc3_is_usb31(...))
?
> + mode = USB_DR_MODE_PERIPHERAL;
> }
>
> if (mode != dwc->dr_mode) {
> --
> 2.11.0
>
> --
> 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
--
With Best Regards,
Andy Shevchenko
--
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 v2 3/3] usb: dwc2: Make dwc2_readl/writel functions endianness-agnostic.
On Thu, Jul 26, 2018 at 5:01 PM, Gevorg Sahakyan
wrote:
> Declared dwc2_check_core_endianness() function for dynamicly check
> core endianness.
> Added needs_byte_swap flag to hsotg structure, and depending on
> flag swap value inside dwc2_readl/writel functions.
> +#define swap32(x) (\
> + {typeof(x) x_ = (x); \
> + (((u32)(x_) << 24) & (u32)0xFF00) | \
> + (((u32)(x_) << 8) & (u32)0x00FF) | \
> + (((u32)(x_) >> 8) & (u32)0xFF00) | \
> + (((u32)(x_) >> 24) & (u32)0x000000FF); })
What's wrong with swab32() ?
--
With Best Regards,
Andy Shevchenko
--
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 v2 2/3] usb: dwc2: replace ioread32/iowrite32_rep with dwc2_readl/writel_rep
On Thu, Jul 26, 2018 at 5:00 PM, Gevorg Sahakyan wrote: > dwc2_readl_rep/dwc2_writel_rep functions using readl/writel in a > loop. Why this is better? Any regression or what? -- With Best Regards, Andy Shevchenko -- 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 v3 1/2] usb: dwc3: pci: Supply device properties via driver data
On Thu, 2018-07-26 at 14:50 +0300, Felipe Balbi wrote: > Andy Shevchenko writes: > > > For now all PCI enumerated dwc3 devices require some properties > > to be present. This allows us to unconditionally append them and > > supply > > via driver_data. > > > > No functional change intended. > > > > Signed-off-by: Andy Shevchenko > > both applied, Thanks! > but I removed your macro below since that only adds > obfuscation and breaks the simple use-case of navigating with the help > of ctags/etags. No hard feelings! -- Andy Shevchenko Intel Finland Oy -- 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 v3 2/2] usb: dwc3: pci: Intel Merrifield can be host
On Intel Edison board the OTG function is enabled, thus,
USB can switch to the host mode.
Allow that by changing dr_mode property to "otg" for Intel Merrifield.
Signed-off-by: Andy Shevchenko
---
drivers/usb/dwc3/dwc3-pci.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index cc4db9a06505..88b800ab6419 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -107,6 +107,12 @@ static const struct property_entry
dwc3_pci_intel_properties[] = {
{}
};
+static const struct property_entry dwc3_pci_mrfld_properties[] = {
+ PROPERTY_ENTRY_STRING("dr_mode", "otg"),
+ PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
+ {}
+};
+
static const struct property_entry dwc3_pci_amd_properties[] = {
PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
PROPERTY_ENTRY_U8("snps,lpm-nyet-threshold", 0xf),
@@ -293,7 +299,7 @@ static void dwc3_pci_remove(struct pci_dev *pci)
static const struct pci_device_id dwc3_pci_id_table[] = {
DWC3_PCI_DEV(INTEL, INTEL_BSW, dwc3_pci_intel_properties),
DWC3_PCI_DEV(INTEL, INTEL_BYT, dwc3_pci_intel_properties),
- DWC3_PCI_DEV(INTEL, INTEL_MRFLD,dwc3_pci_intel_properties),
+ DWC3_PCI_DEV(INTEL, INTEL_MRFLD,dwc3_pci_mrfld_properties),
DWC3_PCI_DEV(INTEL, INTEL_SPTLP,dwc3_pci_intel_properties),
DWC3_PCI_DEV(INTEL, INTEL_SPTH, dwc3_pci_intel_properties),
DWC3_PCI_DEV(INTEL, INTEL_BXT, dwc3_pci_intel_properties),
--
2.18.0
--
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 v3 1/2] usb: dwc3: pci: Supply device properties via driver data
For now all PCI enumerated dwc3 devices require some properties
to be present. This allows us to unconditionally append them and supply
via driver_data.
No functional change intended.
Signed-off-by: Andy Shevchenko
---
drivers/usb/dwc3/dwc3-pci.c | 107 +---
1 file changed, 50 insertions(+), 57 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index b29f031dfc23..cc4db9a06505 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -101,52 +101,37 @@ static int dwc3_byt_enable_ulpi_refclock(struct pci_dev
*pci)
return 0;
}
+static const struct property_entry dwc3_pci_intel_properties[] = {
+ PROPERTY_ENTRY_STRING("dr_mode", "peripheral"),
+ PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
+ {}
+};
+
+static const struct property_entry dwc3_pci_amd_properties[] = {
+ PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
+ PROPERTY_ENTRY_U8("snps,lpm-nyet-threshold", 0xf),
+ PROPERTY_ENTRY_BOOL("snps,u2exit_lfps_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,u2ss_inp3_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,req_p1p2p3_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,del_p1p2p3_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,del_phy_power_chg_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,lfps_filter_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,rx_detect_poll_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,tx_de_emphasis_quirk"),
+ PROPERTY_ENTRY_U8("snps,tx_de_emphasis", 1),
+ /* FIXME these quirks should be removed when AMD NL tapes out */
+ PROPERTY_ENTRY_BOOL("snps,disable_scramble_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,dis_u3_susphy_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,dis_u2_susphy_quirk"),
+ PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
+ {}
+};
+
static int dwc3_pci_quirks(struct dwc3_pci *dwc)
{
- struct platform_device *dwc3 = dwc->dwc3;
struct pci_dev *pdev = dwc->pci;
- if (pdev->vendor == PCI_VENDOR_ID_AMD &&
- pdev->device == PCI_DEVICE_ID_AMD_NL_USB) {
- struct property_entry properties[] = {
- PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
- PROPERTY_ENTRY_U8("snps,lpm-nyet-threshold", 0xf),
- PROPERTY_ENTRY_BOOL("snps,u2exit_lfps_quirk"),
- PROPERTY_ENTRY_BOOL("snps,u2ss_inp3_quirk"),
- PROPERTY_ENTRY_BOOL("snps,req_p1p2p3_quirk"),
- PROPERTY_ENTRY_BOOL("snps,del_p1p2p3_quirk"),
- PROPERTY_ENTRY_BOOL("snps,del_phy_power_chg_quirk"),
- PROPERTY_ENTRY_BOOL("snps,lfps_filter_quirk"),
- PROPERTY_ENTRY_BOOL("snps,rx_detect_poll_quirk"),
- PROPERTY_ENTRY_BOOL("snps,tx_de_emphasis_quirk"),
- PROPERTY_ENTRY_U8("snps,tx_de_emphasis", 1),
- /*
-* FIXME these quirks should be removed when AMD NL
-* tapes out
-*/
- PROPERTY_ENTRY_BOOL("snps,disable_scramble_quirk"),
- PROPERTY_ENTRY_BOOL("snps,dis_u3_susphy_quirk"),
- PROPERTY_ENTRY_BOOL("snps,dis_u2_susphy_quirk"),
- PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
- { },
- };
-
- return platform_device_add_properties(dwc3, properties);
- }
-
if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
- int ret;
-
- struct property_entry properties[] = {
- PROPERTY_ENTRY_STRING("dr_mode", "peripheral"),
- PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
- { }
- };
-
- ret = platform_device_add_properties(dwc3, properties);
- if (ret < 0)
- return ret;
-
if (pdev->device == PCI_DEVICE_ID_INTEL_BXT ||
pdev->device == PCI_DEVICE_ID_INTEL_BXT_M) {
guid_parse(PCI_INTEL_BXT_DSM_GUID, >guid);
@@ -155,6 +140,7 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
if (pdev->device == PCI_DEVICE_ID_INTEL_BYT) {
struct gpio_desc *gpio;
+ int ret;
/* On BYT the FW does not always enable the refclock */
ret = dwc3_byt_enable_ulpi_refclock(pdev);
@@ -216,9 +
Re: [PATCH 4/8] usb: dwc3: implement stream transfer timeout
On Wed, Jul 25, 2018 at 6:14 PM, Anurag Kumar Vulisha wrote: >>> +/* >>> + * Timeout value in msecs used by stream_timeout_timer when streams >>> +are enabled */ >>> +#define STREAM_TIMEOUT 50 >> >>Perhaps, STREAM_TIMEOUT_MS 50 >> >>Dunno about this driver, but it's a usual practice to help reader with >>understanding >>code on the first glance. >> > > Since I have mentioned "msecs" in comment above describing the STREAM_TIMEOUT, > thought it would suffice. But if you feel I should change it, I will fix it > in v2 But you didn't put that comment before each occurrence of this constant in the code, right? That's what I'm talking about. If reader looks into the code, there is no need to understand the order of the value for timeout, since units are part of the name. Of course, you may ignore this comment. -- With Best Regards, Andy Shevchenko -- 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 4/8] usb: dwc3: implement stream transfer timeout
On Wed, Jul 25, 2018 at 2:51 PM, Anurag Kumar Vulisha wrote: > According to dwc3 databook when streams are used, it may be possible > for the host and device become out of sync, where device may wait for > host to issue prime transcation and host may wait for device to issue > erdy. To avoid such deadlock, timeout needs to be implemented. After > timeout occurs, device will first stop transfer and restart the transfer > again. This patch does the same. > +/* > + * Timeout value in msecs used by stream_timeout_timer when streams are > enabled > + */ > +#define STREAM_TIMEOUT 50 Perhaps, STREAM_TIMEOUT_MS 50 Dunno about this driver, but it's a usual practice to help reader with understanding code on the first glance. -- With Best Regards, Andy Shevchenko -- 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 v2 3/3] dwc3: Intel Merrifield can be host
On Intel Edison board the OTG function is enabled, thus,
USB can switch to the host mode.
Allow that by changing dr_mode property to "otg" for Intel Merrifield.
Signed-off-by: Andy Shevchenko
---
drivers/usb/dwc3/dwc3-pci.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 9d3fff91e1bc..ec43fe81ad2a 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -82,6 +82,12 @@ static const struct property_entry
dwc3_pci_intel_properties[] = {
{}
};
+static const struct property_entry dwc3_pci_mrfld_properties[] = {
+ PROPERTY_ENTRY_STRING("dr_mode", "otg"),
+ PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
+ {}
+};
+
static const struct property_entry dwc3_pci_amd_properties[] = {
PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
PROPERTY_ENTRY_U8("snps,lpm-nyet-threshold", 0xf),
@@ -258,7 +264,7 @@ static const struct pci_device_id dwc3_pci_id_table[] = {
DWC3_PCI_DEV(SYNOPSYS, SYNOPSYS_HAPSUSB31,
dwc3_pci_synopsys_properties),
DWC3_PCI_DEV(INTEL, INTEL_BSW, dwc3_pci_intel_properties),
DWC3_PCI_DEV(INTEL, INTEL_BYT, dwc3_pci_intel_properties),
- DWC3_PCI_DEV(INTEL, INTEL_MRFLD, dwc3_pci_intel_properties),
+ DWC3_PCI_DEV(INTEL, INTEL_MRFLD, dwc3_pci_mrfld_properties),
DWC3_PCI_DEV(INTEL, INTEL_SPTLP, dwc3_pci_intel_properties),
DWC3_PCI_DEV(INTEL, INTEL_SPTH, dwc3_pci_intel_properties),
DWC3_PCI_DEV(INTEL, INTEL_BXT, dwc3_pci_intel_properties),
--
2.18.0
--
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 v2 1/3] dwc3: Describe 'wakeup_work' field of struct dwc3_pci
Describe 'wakeup_work' field of struct dwc3_pci to avoid a warning:
drivers/usb/dwc3/dwc3-pci.c:59: warning: Function parameter or member
'wakeup_work' not described in 'dwc3_pci'
Signed-off-by: Andy Shevchenko
---
drivers/usb/dwc3/dwc3-pci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index f57e7c94b8e5..3958b7ae6588 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -47,6 +47,7 @@
* @pci: our link to PCI bus
* @guid: _DSM GUID
* @has_dsm_for_pm: true for devices which need to run _DSM on runtime PM
+ * @wakeup_work: work for asynchronous resume
*/
struct dwc3_pci {
struct platform_device *dwc3;
--
2.18.0
--
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 v1 2/2] dwc3: Supply device properties via driver data
For now all PCI enumerated dwc3 devices require some properties
to be present. This allows us to unconditionally append them and supply
via driver_data.
No functional change intended.
Signed-off-by: Andy Shevchenko
---
drivers/usb/dwc3/dwc3-pci.c | 145 +++-
1 file changed, 61 insertions(+), 84 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 3958b7ae6588..9d3fff91e1bc 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -68,52 +68,45 @@ static const struct acpi_gpio_mapping acpi_dwc3_byt_gpios[]
= {
{ },
};
+static const struct property_entry dwc3_pci_synopsys_properties[] = {
+ PROPERTY_ENTRY_BOOL("snps,usb3_lpm_capable"),
+ PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
+ PROPERTY_ENTRY_BOOL("snps,dis_enblslpm_quirk"),
+ PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
+ {}
+};
+
+static const struct property_entry dwc3_pci_intel_properties[] = {
+ PROPERTY_ENTRY_STRING("dr_mode", "peripheral"),
+ PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
+ {}
+};
+
+static const struct property_entry dwc3_pci_amd_properties[] = {
+ PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
+ PROPERTY_ENTRY_U8("snps,lpm-nyet-threshold", 0xf),
+ PROPERTY_ENTRY_BOOL("snps,u2exit_lfps_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,u2ss_inp3_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,req_p1p2p3_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,del_p1p2p3_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,del_phy_power_chg_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,lfps_filter_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,rx_detect_poll_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,tx_de_emphasis_quirk"),
+ PROPERTY_ENTRY_U8("snps,tx_de_emphasis", 1),
+ /* FIXME these quirks should be removed when AMD NL tapes out */
+ PROPERTY_ENTRY_BOOL("snps,disable_scramble_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,dis_u3_susphy_quirk"),
+ PROPERTY_ENTRY_BOOL("snps,dis_u2_susphy_quirk"),
+ PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
+ {}
+};
+
static int dwc3_pci_quirks(struct dwc3_pci *dwc)
{
- struct platform_device *dwc3 = dwc->dwc3;
struct pci_dev *pdev = dwc->pci;
- if (pdev->vendor == PCI_VENDOR_ID_AMD &&
- pdev->device == PCI_DEVICE_ID_AMD_NL_USB) {
- struct property_entry properties[] = {
- PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
- PROPERTY_ENTRY_U8("snps,lpm-nyet-threshold", 0xf),
- PROPERTY_ENTRY_BOOL("snps,u2exit_lfps_quirk"),
- PROPERTY_ENTRY_BOOL("snps,u2ss_inp3_quirk"),
- PROPERTY_ENTRY_BOOL("snps,req_p1p2p3_quirk"),
- PROPERTY_ENTRY_BOOL("snps,del_p1p2p3_quirk"),
- PROPERTY_ENTRY_BOOL("snps,del_phy_power_chg_quirk"),
- PROPERTY_ENTRY_BOOL("snps,lfps_filter_quirk"),
- PROPERTY_ENTRY_BOOL("snps,rx_detect_poll_quirk"),
- PROPERTY_ENTRY_BOOL("snps,tx_de_emphasis_quirk"),
- PROPERTY_ENTRY_U8("snps,tx_de_emphasis", 1),
- /*
-* FIXME these quirks should be removed when AMD NL
-* tapes out
-*/
- PROPERTY_ENTRY_BOOL("snps,disable_scramble_quirk"),
- PROPERTY_ENTRY_BOOL("snps,dis_u3_susphy_quirk"),
- PROPERTY_ENTRY_BOOL("snps,dis_u2_susphy_quirk"),
- PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
- { },
- };
-
- return platform_device_add_properties(dwc3, properties);
- }
-
if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
- int ret;
-
- struct property_entry properties[] = {
- PROPERTY_ENTRY_STRING("dr_mode", "peripheral"),
- PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
- { }
- };
-
- ret = platform_device_add_properties(dwc3, properties);
- if (ret < 0)
- return ret;
-
if (pdev->device == PCI_DEVICE_ID_INTEL_BXT ||
pdev->device == PCI_DEVICE_ID_INTEL_BXT_M) {
guid_parse(PCI_INTEL_BXT_DSM_GUID, >guid);
@@ -122,6 +115,7 @@ static int dwc3
[PATCH v1 1/2] dwc3: Describe 'wakeup_work' field of struct dwc3_pci
Describe 'wakeup_work' field of struct dwc3_pci to avoid a warning:
drivers/usb/dwc3/dwc3-pci.c:59: warning: Function parameter or member
'wakeup_work' not described in 'dwc3_pci'
Signed-off-by: Andy Shevchenko
---
drivers/usb/dwc3/dwc3-pci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index f57e7c94b8e5..3958b7ae6588 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -47,6 +47,7 @@
* @pci: our link to PCI bus
* @guid: _DSM GUID
* @has_dsm_for_pm: true for devices which need to run _DSM on runtime PM
+ * @wakeup_work: work for asynchronous resume
*/
struct dwc3_pci {
struct platform_device *dwc3;
--
2.18.0
--
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 4/5] USB: serial: cp210x: generalise CP2102N line-speed handling
On Wed, Jul 18, 2018 at 3:37 PM, Johan Hovold wrote: > On Wed, Jul 18, 2018 at 03:34:34PM +0300, Andy Shevchenko wrote: >> On Wed, Jul 18, 2018 at 3:25 PM, Johan Hovold wrote: >> Looks like ping-pong type of changes. >> I think the factoring of this particular piece of code can be done in >> patch 3 in somewhat similar way. > > Indeed, and it was done like this on purpose this time to save time (and > I did not want to rewrite Karoly's patch beyond recognition). Ah, it explains. -- With Best Regards, Andy Shevchenko -- 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 4/5] USB: serial: cp210x: generalise CP2102N line-speed handling
On Wed, Jul 18, 2018 at 3:25 PM, Johan Hovold wrote:
> The CP2102N equations for determining the actual baud rate can be used
> also for other device types, so let's factor it out.
>
> Note that this removes the now unused cp210x_is_cp2102n() helper.
> +static speed_t cp210x_get_actual_rate(struct usb_serial *serial, speed_t
> baud)
> +{
> + struct cp210x_serial_private *priv = usb_get_serial_data(serial);
> + unsigned int prescale = 1;
> + unsigned int div;
> +
> + baud = clamp(baud, 300u, priv->max_speed);
> +
> + if (baud <= 365)
> + prescale = 4;
> +
> + div = DIV_ROUND_CLOSEST(4800, 2 * prescale * baud);
> + baud = 4800 / (2 * prescale * div);
> +
> + return baud;
> +}
> - if (cp210x_is_cp2102n(serial)) {
> - int clk_div;
> - int prescaler;
> -
> - baud = clamp(baud, 300u, priv->max_speed);
> - prescaler = (baud <= 365) ? 4 : 1;
> - clk_div = DIV_ROUND_CLOSEST(4800, 2 * prescaler * baud);
> - baud = 4800 / (2 * prescaler * clk_div);
> - }
Looks like ping-pong type of changes.
I think the factoring of this particular piece of code can be done in
patch 3 in somewhat similar way.
--
With Best Regards,
Andy Shevchenko
--
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 v5] USB: serial: ftdi_sio: Add MTP NVM support
On Tue, Jun 26, 2018 at 3:57 PM, Andy Shevchenko wrote: Just noticed you have sent v6. I'm pretty fine with it, thanks! -- With Best Regards, Andy Shevchenko -- 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 v5] USB: serial: ftdi_sio: Add MTP NVM support
On Tue, Jun 26, 2018 at 2:51 PM, Loic Poulain wrote:
> On 26 June 2018 at 13:02, Andy Shevchenko wrote:
>> On Mon, Jun 25, 2018 at 3:35 PM, Loic Poulain
>> wrote:
>>> + unsigned char *buf = val;
>>
>> Btw, not sure why you need this now...
>
> Not needed indeed, just wanted to avoid arithmetic on void pointer,
> but I can cast instead.
No need to cast either.
We rely (?) on GCC to allow pointer arithmetic on void pointers to
behave as char pointers.
(?) I don't know if it's deliberate choice, though a lot of code uses
pointer arithmetic against void pointers such if it would be a char
pointers.
In case you would like to keep it, the temporary variable looks the
best way (no ugly castings, please).
>>> +
>>> + if (bytes % 2) /* 16-bit eeprom */
>>> + return -EINVAL;
>>> +
>>> + while (bytes) {
>>> + int rv;
>>> +
>>> + rv = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
>>> +FTDI_SIO_WRITE_EEPROM_REQUEST,
>>> +FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE,
>>> +get_unaligned_le16(buf), off / 2, NULL,
>>> +0, WDR_TIMEOUT);
>>> + if (rv < 0)
>>> + return rv;
>>> +
>>> + off += 2;
>>> + buf += 2;
>>> + bytes -= 2;
>>> + }
--
With Best Regards,
Andy Shevchenko
--
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 v5] USB: serial: ftdi_sio: Add MTP NVM support
On Mon, Jun 25, 2018 at 3:35 PM, Loic Poulain wrote:
> Most of FTDI's devices have an EEPROM which records FTDI devices
> configuration setting (e.g. the VID, PID, I/O config...) and user
> data. For example, FT232R and FTX chips have 128-byte and 2048-byte
> internal EEPROM respectively.
>
> This patch adds support for FTDI EEPROM read/write via USB control
> transfers and register a new nvm device to the nvmem core.
>
> This permits to expose the EEPROM as a sysfs file, allowing userspace
> to read/modify FTDI configuration and its user data without having to
> rely on a specific userspace USB driver.
>
> Moreover, any upcoming new tentative to add CBUS GPIO support could
> integrate CBUS EEPROM configuration reading in order to determine
> which of the CBUS pins are available as GPIO.
>
FWIW,
Reviewed-by: Andy Shevchenko
One nit below, though.
> Signed-off-by: Loic Poulain
> ---
> v2: Use ifdef instead of IS_ENABLED
> error message in case of nvmem registering failure
> Fix space/tab in Kconfig
> v3: Make nvmem a child of the usb dev instead of the serial port
> Add macros defining eeprom sizes
> Check read/write size is a nultiple of the eeprom word-size
> Remove useless change in Kconfig
> v4: Reword commit message
> Remove default-yes from Kconfig
> Change includes ordering
> Use default linux size defines
> Use get_unaligned_le16 helper
> Prepend EEPROM functions with ftdi_
> Error message in ftdi_eeprom_register()
> v5: Fix missing linux/sizes header
>
> drivers/usb/serial/Kconfig| 10
> drivers/usb/serial/ftdi_sio.c | 119
> ++
> drivers/usb/serial/ftdi_sio.h | 28 ++
> 3 files changed, 157 insertions(+)
>
> diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
> index 533f127..5747562 100644
> --- a/drivers/usb/serial/Kconfig
> +++ b/drivers/usb/serial/Kconfig
> @@ -181,6 +181,16 @@ config USB_SERIAL_FTDI_SIO
> To compile this driver as a module, choose M here: the
> module will be called ftdi_sio.
>
> +config USB_SERIAL_FTDI_SIO_NVMEM
> + bool "FTDI MTP non-volatile memory support"
> + depends on USB_SERIAL_FTDI_SIO
> + select NVMEM
> + help
> + Say yes here to add support for the MTP non-volatile memory
> + present on FTDI. Most of FTDI's devices have an EEPROM which
> + records FTDI device's configuration setting as well as user
> + data.
> +
> config USB_SERIAL_VISOR
> tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
> help
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index 7ea221d..75a35a8 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -37,9 +37,13 @@
> #include
> #include
> #include
> +#include
> +#include
> #include
> #include
> #include
> +#include
> +
I would put it rather as follows
+#include
... // something else is still in order here?
+#include
#include
#include
#include
#include
#include
#include
+
+#include
+
// also note another blank line as divider.
> #include "ftdi_sio.h"
> #include "ftdi_sio_ids.h"
>
> @@ -73,6 +77,8 @@ struct ftdi_private {
> unsigned int latency; /* latency setting in use */
> unsigned short max_packet_size;
> struct mutex cfg_lock; /* Avoid mess by parallel calls of config
> ioctl() and change_speed() */
> +
> + struct nvmem_device *eeprom;
> };
>
> /* struct ftdi_sio_quirk is used by devices requiring special attention. */
> @@ -1529,6 +1535,112 @@ static int get_lsr_info(struct usb_serial_port *port,
> return 0;
> }
>
> +#ifdef CONFIG_USB_SERIAL_FTDI_SIO_NVMEM
> +
> +static int ftdi_write_eeprom(void *priv, unsigned int off, void *val,
> +size_t bytes)
> +{
> + struct usb_serial_port *port = priv;
> + struct usb_device *udev = port->serial->dev;
> + unsigned char *buf = val;
Btw, not sure why you need this now...
> +
> + if (bytes % 2) /* 16-bit eeprom */
> + return -EINVAL;
> +
> + while (bytes) {
> + int rv;
> +
> + rv = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> +FTDI_SIO_WRITE_EEPROM_REQUEST,
> +FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE,
> +get_unaligned_le16(buf), off / 2, NULL,
> +0, WDR_TIMEOUT);
> + if (
Re: [PATCH v3] USB: serial: ftdi_sio: Add MTP NVM support
On Fri, Jun 22, 2018 at 5:22 PM, Loic Poulain wrote:
> Most of FTDI's devices have an EEPROM which records FTDI devices
> configuration setting (e.g. the VID, PID, I/O config...) and user
> data.
> FT230R chip integrates a 128-byte eeprom, FT230X a 2048-byte
> eeprom...
This sounds unfinished. What is the continuation of the sentence?
>
> This patch adds support for FTDI EEPROM read/write via USB control
> transfers and register a new nvm device to the nvmem core.
>
> This permits to expose the eeprom as a sysfs file, allowing userspace
> to read/modify FTDI configuration and its user data without having to
> rely on a specific userspace USB driver.
>
> Moreover, any upcoming new tentative to add CBUS GPIO support could
> integrate CBUS EEPROM configuration reading in order to determine
> which of the CBUS pins are available as GPIO.
In some cases you are using EEPROM, in the rest, eeprom. What the difference?
> +config USB_SERIAL_FTDI_SIO_NVMEM
> + bool "FTDI MTP non-volatile memory support"
> + depends on USB_SERIAL_FTDI_SIO
> + select NVMEM
> + default y
First of all, I didn't find an evidence why it should be y...
> + help
> + Say yes here to add support for the MTP non-volatile memory
> + present on FTDI. Most of FTDI's devices have an EEPROM which
> + records FTDI device's configuration setting as well as user
> + data.
...second, the help semantics is inconsistent with default.
> #include
> #include
> #include
> +#include
Perhaps squeeze it into most ordered part and keep there an order?
> +#define EEPROM_SZ_FTX 2048 /* cf FTDI AN_201 */
> +#define EEPROM_SZ_FT232RL 128 /* cf FTDI AN_121 */
These defines looks too particular, shouldn't be this a part of driver
data / platform data / device properties?
Also, above I think is already defined in a common way in linux/sizes.h.
> +static int write_eeprom(void *priv, unsigned int off, void *_val, size_t
> bytes)
Leading _ in the name looks weird here, since it's not a macro.
(and inconsistent with below function definitions)
> + val = buf[0] + (buf[1] << 8);
get_unaligned_le16() ?
> + if (register_eeprom(port)) {
> + dev_err(>dev, "Unable to register FTDI EEPROM\n");
> + /* non-fatal */
Doesn't register_eeprom() have some error messaging already?
Btw, can we rename it to be less generic, like adding a prefix?
> + }
--
With Best Regards,
Andy Shevchenko
--
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 2/3] usb: dwc3: pci: Enable ULPI Refclk on platforms where the firmware doesnot
On Thu, Jun 7, 2018 at 4:42 PM, Hans de Goede wrote: >>> + msleep(100); >> >> >> This has to be explained. > > > Erm, this comes 1:1 from Intels Android X86 patches I've no > idea why it is there, I believe it is better to leave this > uncommented then making something up. The above would be a good candidate /* This comes from the Intel Android x86 tree w/o any explanation */ -- With Best Regards, Andy Shevchenko -- 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 2/3] usb: dwc3: pci: Enable ULPI Refclk on platforms where the firmware doesnot
On Thu, Jun 7, 2018 at 1:38 PM, Hans de Goede wrote:
> On some Bay Trail (BYT) systems the firmware does not enable the
> ULPI Refclk.
>
> This commit adds a helper which checks and if necessary enabled the Refclk
> and calls this helper for BYT machines.
> +static void dwc3_pci_enable_ulpi_refclock(struct pci_dev *pci)
> +{
> + void __iomem*reg;
> + struct resource res;
> + struct device *dev = >dev;
> + u32 value;
> +
> + res.start = pci_resource_start(pci, 1);
> + res.end = pci_resource_end(pci, 1);
> + res.name= "dwc_usb3_bar1";
> + res.flags = IORESOURCE_MEM;
> +
> + reg = devm_ioremap_resource(dev, );
> + if (IS_ERR(reg)) {
> + dev_err(dev, "cannot check GP_RWREG1 to assert ulpi
> refclock\n");
> + return;
> + }
I'm not sure I understand what's wrong with simple
pci_iomap() & Co (perhaps pcim_iomap() / pcim_iomap_regions() and others)
> +
> + value = readl(reg + GP_RWREG1);
> + if (!(value & GP_RWREG1_ULPI_REFCLK_DISABLE))
> + return; /* ULPI refclk already enabled */
> +
> + dev_warn(dev, "ULPI refclock is disabled from the BIOS, enabling
> it\n");
> + value &= ~GP_RWREG1_ULPI_REFCLK_DISABLE;
> + writel(value, reg + GP_RWREG1);
> + msleep(100);
This has to be explained.
> +}
--
With Best Regards,
Andy Shevchenko
--
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/3] usb: dwc3: pci: Add GPIO lookup table on platforms without ACPI GPIO resources
On Thu, Jun 7, 2018 at 1:38 PM, Hans de Goede wrote:
> Bay Trail / BYT SoCs do not have a builtin device-mode phy, instead
> they require an external ULPI phy for device-mode.
>
> Only some BYT devices have an external phy, but even on those devices
> device-mode is not working because the dwc3 does not see the phy.
>
> The problem is that the ACPI fwnode for the dwc3 does not contain the
> expected GPIO resources for the GPIOs connected to the chip-select and
> reset pins of the phy.
>
> I've found the workaround which some Android x86 kernels use for this:
> https://github.com/BORETS24/Kernel-for-Asus-Zenfone-2/blob/master/arch/x86/platform/intel-mid/device_libs/pci/platform_usb_otg.c
> Which boils down to hardcoding the GPIOs for these devices.
>
> This commit adds a gpiod_lookup_table adding the mappings from Android,
> and installs this table on BYT system with their BIOS vendor DMI string
> set to "INSYDE Corp.". This seems to indicate that a (mostly) unmodified
> version of the reference design BIOS is used. 3 out of the 20 BYT tablets
> which I have for testing have an external ULPI phy and all 3 models use
> this vendor string.
> + vendor = dmi_get_system_info(DMI_BIOS_VENDOR);
> + if (vendor && strcmp(vendor, "INSYDE Corp.") == 0) {
> + gpiod_add_lookup_table(_bytcr_gpios);
> + dwc->gpio_mapping_added = true;
> + }
I understand that my proposal might sound a bit overkill, though, I
would rather go with normal dmi tables and call back there.
P.S. Though, I would be okay with this as long as Felipe fine.
--
With Best Regards,
Andy Shevchenko
--
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 v2 2/3] platform: x86: intel_cht_int33fe: Fix dependencies
On Wed, 2018-05-23 at 17:37 +0300, Heikki Krogerus wrote: > The driver will not probe unless bq24190 is loaded, so > making it a dependency. > Acked-by: Andy Shevchenko <andriy.shevche...@linux.intel.com> assuming it will go through some other tree. > Signed-off-by: Heikki Krogerus <heikki.kroge...@linux.intel.com> > Cc: Wolfram Sang <w...@the-dreams.de> > Cc: Darren Hart <dvh...@infradead.org> > Cc: Andy Shevchenko <andriy.shevche...@linux.intel.com> > --- > drivers/i2c/busses/Kconfig | 3 +-- > drivers/platform/x86/Kconfig | 4 ++-- > 2 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 99edffae27f6..4f8df2ec87b1 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -202,8 +202,7 @@ config I2C_CHT_WC > > Note this controller is hooked up to a TI bq24292i charger- > IC, > combined with a FUSB302 Type-C port-controller as such it > is advised > - to also select CONFIG_CHARGER_BQ24190=m and > CONFIG_TYPEC_FUSB302=m > - (the fusb302 driver currently is in drivers/staging). > + to also select CONFIG_TYPEC_FUSB302=m. > > config I2C_NFORCE2 > tristate "Nvidia nForce2, nForce3 and nForce4" > diff --git a/drivers/platform/x86/Kconfig > b/drivers/platform/x86/Kconfig > index 566644bb496a..f27cb186437d 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -866,6 +866,7 @@ config ACPI_CMPC > config INTEL_CHT_INT33FE > tristate "Intel Cherry Trail ACPI INT33FE Driver" > depends on X86 && ACPI && I2C && REGULATOR > + depends on CHARGER_BQ24190=y || (CHARGER_BQ24190=m && m) > ---help--- > This driver add support for the INT33FE ACPI device found > on > some Intel Cherry Trail devices. > @@ -877,8 +878,7 @@ config INTEL_CHT_INT33FE > i2c drivers for these chips can bind to the them. > > If you enable this driver it is advised to also select > - CONFIG_TYPEC_FUSB302=m, CONFIG_CHARGER_BQ24190=m and > - CONFIG_BATTERY_MAX17042=m. > + CONFIG_TYPEC_FUSB302=m and CONFIG_BATTERY_MAX17042=m. > > config INTEL_INT0002_VGPIO > tristate "Intel ACPI INT0002 Virtual GPIO driver" -- Andy Shevchenko <andriy.shevche...@linux.intel.com> Intel Finland Oy -- 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 v5] usb: gadget: udc: renesas_usb3: Add register of usb role switch
On Fri, May 25, 2018 at 10:12 AM, Yoshihiro Shimoda
<yoshihiro.shimoda...@renesas.com> wrote:
> -static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)
Wouldn't be better to choose another name for a new function?
> + struct renesas_usb3 *usb3 = container_of(work, struct renesas_usb3,
> +role_work);
Matter of style, though I would rather put entire container_of() on
the next line (see for the existing style in the module and use it).
> + /* This device_attach() might sleep */
> + if (device_attach(host) < 0)
> + dev_err(dev, "device_attach(usb3_port) failed\n");
can't be "usb3_port" part derived from the host variable somehow and
to some extend?
> + usb3->role_sw = usb_role_switch_register(>dev,
> + _usb3_role_switch_desc);
> + if (!IS_ERR(usb3->role_sw)) {
> + usb3->host_dev = usb_of_get_companion_dev(>dev);
Hmm... Can it possible return -EPROBE_DEFER? If so, would it be better
to use other approach to handle it?
> + if (IS_ERR_OR_NULL(usb3->host_dev)) {
> + /* If not found, this driver will not use a role sw */
> + usb_role_switch_unregister(usb3->role_sw);
> + usb3->role_sw = NULL;
> + }
> + } else {
> + usb3->role_sw = NULL;
> + }
--
With Best Regards,
Andy Shevchenko
--
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 01/33] usb: phy: use match_string() helper
On Mon, May 21, 2018 at 2:57 PM, Yisheng Xie <xieyishe...@huawei.com> wrote: > match_string() returns the index of an array for a matching string, > which can be used intead of open coded variant. > - int err, i; > + int ret; int err; would still work. -- With Best Regards, Andy Shevchenko -- 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 03/12] usb: usbtmc: Add ioctls to set/get usb timeout
On Thu, May 17, 2018 at 8:03 PM, Guido Kiener <gu...@kiener-muenchen.de> wrote:
> Add ioctls USBTMC_IOCTL_GET_TIMEOUT / USBTMC_IOCTL_SET_TIMEOUT to
> get/set I/O timeout for specific file handle.
> +static int usbtmc_ioctl_get_timeout(struct usbtmc_file_data *file_data,
> + void __user *arg)
> +{
> + __u32 timeout;
> +
> + timeout = file_data->timeout;
Why do you need __u32 on kernel side?
Would plain u32 work?
> +
> + return put_user(timeout, (__u32 __user *)arg);
> +}
> +static int usbtmc_ioctl_set_timeout(struct usbtmc_file_data *file_data,
> + void __user *arg)
> +{
> + __u32 timeout;
Ditto.
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
--
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: [RFC PATCH v3 5/5] usb: typec: tcpm: Support for Alternate Modes
On Fri, May 11, 2018 at 4:18 PM, Heikki Krogerus <heikki.kroge...@linux.intel.com> wrote: > This adds more complete handling of VDMs and registration of > partner alternate modes, and introduces callbacks for > alternate mode operations. > > Only DFP role is supported for now. > + for (i = 0; i < cnt; i++) > + p[i] = le32_to_cpu(payload[i]); I would recommend to consider to use le32_to_cpu_array(). Though, actually we have slightly different API for BE and LE cases. For LE existing would be renamed to rather le32_to_cpus_array() and establishing the former one in the similar way how be32_to_cpu_array() is implemented. -- With Best Regards, Andy Shevchenko -- 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: role: rcar-usb3-role-switch: add support for R-Car SoCs
On Tue, Apr 10, 2018 at 3:03 PM, Yoshihiro Shimoda
<yoshihiro.shimoda...@renesas.com> wrote:
> This patch adds role switch support for R-Car SoCs. Some R-Car SoCs
> (e.g. R-Car H3) have USB 3.0 dual-role device controller which has
> the USB 3.0 xHCI host and Renesas USB 3.0 peripheral.
>
> Unfortunately, the mode change register contains the USB 3.0 peripheral
> controller side only. So, the USB 3.0 peripheral driver (renesas_usb3)
> manages this register. However, in peripheral mode, the host should
> stop. Also the host hardware needs to reinitialize its own registers
> when the mode changes from peripheral to host mode. Otherwise,
> the host cannot work correctly (e.g. detect a device as high-speed).
>
> To achieve this by a driver, this role switch driver manages
> the mode change register and attach/release the xhci-plat driver.
> The renesas_usb3 udc driver should call devm_of_platform_populate()
> to probe this driver.
> +#include
> +#include
> +#include
> +#include
Do you need this one?
> +#include
> +#include
> +#include
> +#include
> +#include
> +static const struct of_device_id rcar_usb3_role_switch_of_match[] = {
> + { .compatible = "renesas,rcar-usb3-role-switch" },
> + { },
Better to remove comma at terminator line.
> +};
> +MODULE_DEVICE_TABLE(of, rcar_usb3_role_switch_of_match);
> +static int rcar_usb3_role_switch_probe(struct platform_device *pdev)
> +{
> + /* Avoid devm_request_mem_region() calling by devm_ioremap_resource()
> */
Redundant comment.
> + host_node = of_parse_phandle(pdev->dev.of_node, "renesas,host", 0);
> + if (!host_node)
> + return -ENODEV;
> +
> + pdev_host = of_find_device_by_node(host_node);
> + if (!pdev_host)
> + return -ENODEV;
> +
> + of_node_put(host_node);
Isn't what Heikki tried to solve with graphs and stuff like that?
> + dev_info(>dev, "probed\n");
Noise. (Hint: initcall_debug is a good command line option)
> +}
> +#ifdef CONFIG_PM_SLEEP
Instead...
> +static int rcar_usb3_role_switch_suspend(struct device *dev)
...use __maybe_unused annotation.
> +static int rcar_usb3_role_switch_resume(struct device *dev)
Ditto.
> +#endif
> +static struct platform_driver rcar_usb3_role_switch_driver = {
> + .probe = rcar_usb3_role_switch_probe,
> + .remove = rcar_usb3_role_switch_remove,
> + .driver = {
> + .name = "rcar_usb3_role_switch",
> + .pm = _usb3_role_switch_pm_ops,
> + .of_match_table =
> of_match_ptr(rcar_usb3_role_switch_of_match),
Is it possible to have this driver w/o OF? Does it make sense in that case?
Otherwise remove of_match_ptr() call and below modalias.
> + },
> +};
> +MODULE_ALIAS("platform:rcar_usb3_role_switch");
--
With Best Regards,
Andy Shevchenko
--
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 v2] USB: host: sl811: Re-use DEFINE_SHOW_ATTRIBUTE() macro
...instead of open coding file operations followed by custom ->open()
callbacks per each attribute.
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
- fix compile error
drivers/usb/host/sl811-hcd.c | 17 +++--
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/sl811-hcd.c b/drivers/usb/host/sl811-hcd.c
index fa88a903fa2e..5b061e599948 100644
--- a/drivers/usb/host/sl811-hcd.c
+++ b/drivers/usb/host/sl811-hcd.c
@@ -1381,7 +1381,7 @@ static void dump_irq(struct seq_file *s, char *label, u8
mask)
(mask & SL11H_INTMASK_DP) ? " dp" : "");
}
-static int sl811h_show(struct seq_file *s, void *unused)
+static int sl811h_debug_show(struct seq_file *s, void *unused)
{
struct sl811*sl811 = s->private;
struct sl811h_ep*ep;
@@ -1491,25 +1491,14 @@ static int sl811h_show(struct seq_file *s, void *unused)
return 0;
}
-
-static int sl811h_open(struct inode *inode, struct file *file)
-{
- return single_open(file, sl811h_show, inode->i_private);
-}
-
-static const struct file_operations debug_ops = {
- .open = sl811h_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(sl811h_debug);
/* expect just one sl811 per system */
static void create_debug_file(struct sl811 *sl811)
{
sl811->debug_file = debugfs_create_file("sl811h", S_IRUGO,
usb_debug_root, sl811,
- _ops);
+ _debug_fops);
}
static void remove_debug_file(struct sl811 *sl811)
--
2.16.1
--
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: Webcams not recognized on a Dell Latitude 5285 laptop
On Sun, Mar 11, 2018 at 2:38 PM, FRÉDÉRIC PARRENIN <frederic.parre...@univ-grenoble-alpes.fr> wrote: > Dear Oliver and all, > > So I was expecting linux-4.16 to recognize my webcams, thanks to this new PCI > driver Oliver mentioned. > Therefore I installed 4.16-rc4. > Unfortunately, there is still no /dev/video* device > > Any idea what could be done to have these webcams working? I guess you need a driver. Cc: + Sakari, and thus leaving the complete message uncut. > > Thanks, > > Frederic > > >> De: "FRÉDÉRIC PARRENIN" <frederic.parre...@univ-grenoble-alpes.fr> >> À: "Oliver Neukum" <oneu...@suse.com> >> Cc: "linux-usb" <linux-usb@vger.kernel.org> >> Envoyé: Lundi 11 Décembre 2017 16:39:41 >> Objet: Re: Webcams not recognized on a Dell Latitude 5285 laptop > >> > > > it looks like you need the experimental driver posted here > >> > > > https://www.spinics.net/lists/linux-media/msg123268.html > >> > > Thanks for the information. >> >> So, if I understand correctly, this driver will not be included in 4.15, >> >> will >> > > it? >> > > Any idea when this will be included in a release? > >> > I have no idea. Could you contact the original developers? >> > The answer is interesting, but I have no idea. > >> It seems it will be included in the 4.16 release: > >> https://www.mail-archive.com/linux-media@vger.kernel.org/msg122619.html > >> Probably just a bit too late for 4.15. > >> Frederic > > !!! WARNING!!! New email address: frederic.parre...@univ-grenoble-alpes.fr > http://pp.ige-grenoble.fr/pageperso/parrenif/index.html > > - Mail original - >> De: "FRÉDÉRIC PARRENIN" <frederic.parre...@univ-grenoble-alpes.fr> >> À: "Oliver Neukum" <oneu...@suse.com> >> Cc: "linux-usb" <linux-usb@vger.kernel.org> >> Envoyé: Lundi 11 Décembre 2017 16:39:41 >> Objet: Re: Webcams not recognized on a Dell Latitude 5285 laptop > >> > > > it looks like you need the experimental driver posted here > >> > > > https://www.spinics.net/lists/linux-media/msg123268.html > >> > > Thanks for the information. >> >> So, if I understand correctly, this driver will not be included in 4.15, >> >> will >> > > it? >> > > Any idea when this will be included in a release? > >> > I have no idea. Could you contact the original developers? >> > The answer is interesting, but I have no idea. > >> It seems it will be included in the 4.16 release: > >> https://www.mail-archive.com/linux-media@vger.kernel.org/msg122619.html > >> Probably just a bit too late for 4.15. > >> Frederic > -- > 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 -- With Best Regards, Andy Shevchenko -- 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/2] net/usb/ax88179_178a: Use common code in ax88179_chk_eee()
On Sat, Mar 10, 2018 at 8:41 PM, Joe Perches <j...@perches.com> wrote: > On Sat, 2018-03-10 at 19:24 +0100, SF Markus Elfring wrote: >> From: Markus Elfring <elfr...@users.sourceforge.net> >> Date: Sat, 10 Mar 2018 18:22:43 +0100 >> >> Adjust a jump target so that a bit of common code can be better reused >> at the end of this function. > > Please stop mindlessly sending patching Markus. > > How about looking at the code being modified and > thinking about what it does? You have a strong nerve, I gave up on him and would rather simple ban. -- With Best Regards, Andy Shevchenko -- 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: isp1760: Use kasprintf
On Wed, Mar 7, 2018 at 8:08 PM, Himanshu Jha <himanshujha199...@gmail.com> wrote: > Use kasprintf instead of combination of kmalloc and sprintf and > therefore avoid unnecessary computation of string length. > devname = dev_name(isp->dev); Do you still need this temporary variable? > - udc->irqname = kmalloc(strlen(devname) + 7, GFP_KERNEL); > + udc->irqname = kasprintf(GFP_KERNEL, "%s (udc)", devname); Perhaps devname -> dev_name(isp->dev) ? -- With Best Regards, Andy Shevchenko -- 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 v5 01/12] drivers: base: Unified device connection lookup
On Thu, Mar 1, 2018 at 9:28 AM, Heikki Krogerus
<heikki.kroge...@linux.intel.com> wrote:
> Hi,
>
> On Thu, Mar 01, 2018 at 12:56:57AM +, Jun Li wrote:
>> > +struct device *device_find_connection(struct device *dev, const char
>> > +*con_id) {
>> > + return __device_find_connection(dev, con_id, generic_match, NULL); }
>>
>> - return __device_find_connection(dev, con_id, generic_match, NULL);
>> + return __device_find_connection(dev, con_id, NULL, generic_match);
>
> Good catch!
It seems I proposed to put function first parameter followed by opaque
data pointer for it.
In that case it would be exactly like now.
Though, I didn't check if the parameter ordering was changed in the prototype.
--
With Best Regards,
Andy Shevchenko
--
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 v3 00/12] USB Type-C device-connection, mux and switch support
On Mon, Feb 26, 2018 at 1:19 PM, Hans de Goede <hdego...@redhat.com> wrote: > On 26-02-18 11:59, Andy Shevchenko wrote: >> On Mon, Feb 26, 2018 at 11:09 AM, Hans de Goede <hdego...@redhat.com> >> wrote: >> Didn't have time to comment on v2, so here we are: >> you are using in even the same file two styles, i.e. IS_ERR_OR_NULL >> vs. !x || IS_ERR(x) (and negative ones). > Good catch, I only see this in "usb: common: Small class for USB role > switches", > will fix for v4. > >> Reviewed-by: Andy Shevchenko <andy.shevche...@gmail.com> > So with the above fixed I can apply your reviewed-by to the entire series? Right. -- With Best Regards, Andy Shevchenko -- 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 v3 00/12] USB Type-C device-connection, mux and switch support
On Mon, Feb 26, 2018 at 11:09 AM, Hans de Goede <hdego...@redhat.com> wrote: > Hi All, > > Here is version 3 of Heikki's and my USB Type-C device-connection, mux and > switch support series. Version 2 and 3 bring various small code and style > fixes based on review (no major changes). > > Here is the original cover-letter of v1: > > Some devices with an USB Type-C connector have a bunch of muxes > behind that connector which need to be controlled by the kernel (rather > then having them controlled by firmware as on most devices). > > Quite a while back I submitted a patch-series to tie together these muxes > and the Type-C Port Manager (tcpm) code, using the then new drivers/mux > framework. But the way I used the mux framework went against what it was > designed for, so in the end that series got nowhere. > > Heikki Krogerus from Intel, who maintains the USB TYPEC subsystem, has > recently been working on solving the same problem for some boards he is > doing hardware-enablement for. > > Heikki has come up with a number of infrastructure patches for this. > The first one is a new device-connection framework. This solves the > problem of describing non bus device-links on x86 in what in my experience > with this problematic area is a really nice simple, clean and *generic* > way. This could for example in the near future also replace the custom > lookup code in the pwm subsys and the custom pwm_add_table() / > pwm_remove_table() functions. > > The other 3 patches add a framework for the different type of Type-C / > USB "muxes". > > Heikki and I have gone through a number of iterations of these patches > together and we believe these are now ready for merging. Since merging > infrastructure patches without users is not done and Heikki's own use-case > for these is not yet ready for merging, the rest of this series consists > of patches by me to make the Type-C connector found on some Cherry Trail > devices (finally) be able to actually work as an USB port and not just > a charge port. > > The last patch uses the new usb-role-switch framework to also do proper > devcie / host switching on CHT devices with a USB micro AB connector. > This is also a big feature for CHT users, because before this they had > to do a reboot to get an OTG-host cable recognized (on some devices). > > Part of this series is an usb-role-switch driver for the role-switch > found inside the xhci controller on e.g. CHT devices, this is currently > implemented as the generic xhci controller instantiating a platform > child-device for this, since this really is a separate chunk of HW > which happens to sit in the XHCI mmio space. This approach may not be > universally liked, given that in this new series the role-switch driver > is much smaller and does not have any external deps anymore we could > just integrate it into the xhci code if that is preferred. > > About merging this series (once everything is reviewed, etc.), there are > quite some interdependencies in it esp. a lot of the patches depend on > the first patch. Luckily patches 1-10 all apply to subsystems which are > maintained by Greg (most to the USB subsys). Which just leaves patches > 11 and 12 once 1-10 are merged. Greg, can you create an immutable branch > for the platform/x86 and extcon maintainers to merge once this is done? Didn't have time to comment on v2, so here we are: you are using in even the same file two styles, i.e. IS_ERR_OR_NULL vs. !x || IS_ERR(x) (and negative ones). Otherwise, take mine Reviewed-by: Andy Shevchenko <andy.shevche...@gmail.com> > > Regards, > > Hans -- With Best Regards, Andy Shevchenko -- 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: [RFC PATCH] drivers: use 'depends on MFD_SYSCON' instead of 'select MFD_SYSCON'
On Sat, Feb 24, 2018 at 4:50 PM, Masahiro Yamada <yamada.masah...@socionext.com> wrote: > Also, make MFD_SYSCON 'default y' because some defconfig files may > rely on someone select's MFD_SYSCON. I'm not sure this is right thing to do. You basically imply that there always a user of it. -- With Best Regards, Andy Shevchenko -- 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/7] usb: dwc3: pci: Store device properties dynamically
On Thu, Feb 22, 2018 at 1:57 AM, Thinh Nguyen <thinh.ngu...@synopsys.com> wrote:
> On 2/21/2018 6:46 AM, Andy Shevchenko wrote:
>> On Tue, Feb 20, 2018 at 11:12 PM, Thinh Nguyen
>> <thinh.ngu...@synopsys.com> wrote:
>>> On 2/17/2018 7:29 AM, Andy Shevchenko wrote:
>>>> On Fri, Feb 16, 2018 at 11:55 PM, Thinh Nguyen
>>>> <thinh.ngu...@synopsys.com> wrote:
>>>>> Add the ability to add device properties dynamically. Currently, device
>>>>> properties are added to platform device using
>>>>> platform_device_add_properties(). However, this function does not allow
>>>>> adding properties incrementally. It is useful to have this ability when
>>>>> the driver needs to set common device properties across different HW
>>>>
>>>> I'm not sure it's useful anyhow.
>>>>
>>>>> or
>>>>> if IP and FPGA validation test different configurations for different
>>>>> HW.
>>>>
>>>> Shouldn't be a separate stuff for FPGA exclusively?
>>>
>>> Can you clarify/expand your question?
>>
>> FPGA is the one which might have different properties at run time for
>> the same device.
>> So, why do we care on driver / generic level of it?
>>
>> Shouldn't be FPGA manager take care of it (via DT overlays, for example)?
>
> Normally it is handled using DT overlays. But for using FPGA as PCI
> device, I'm not aware of a better solution.
If it's a PCI device, it may utilize PCI (hot plug if you would like
to change IP at run time) with appropriate IDs and stuff, right?
>>>>> Introduce two new functions to do so:
>>>>>* dwc3_pci_add_one_property() - this function adds one property to
>>>>> dwc->properties array and increase its size as needed
>>>>>* dwc3_pci_add_properties() - this function takes a null terminated
>>>>> array of device properties and add them to dwc->properties
>>>>
>>>> So, why you can't use ACPI / DT here?
>>> dwc3_pci_add_properties() is a convenient function that takes statically
>>> allocated array of (quirks) properties for different HW and store them
>>> to dwc->properties. The idea is to add more properties on top of these
>>> required quirks.
>>
>> Yes, I understand that. What's wrong with DT? The built-in device
>> properties have the same nature as usual properties for DT.
>> Whenever driver calls device_property_read_uXX() or alike it would
>> check all property provides for asked one.
>
> I may not have explained fully in my commit message the purpose of this
> change. That's why I think I misinterpreted your previous question.
> With this new debugging feature, we want to delay adding device
> properties until the user initiates it.
So, see above.
When user wants to enable this IP at run time it will be a PCI hot
plug event which makes device appear behind PCI switch.
When device appears it would have it's own VndrID/DevID + sub pair.
Based on that IDs you may apply hard coded quirks (though I am against
quirks in new hardware) as it's done right now.
> Because the driver cannot use
> platform_device_add_properties() to incrementally add properties to
> platform device, we need a way to store properties so they can be added
> in later time.
So what? You don't need that if you do the right things right.
> That's why I added these 2 new functions.
>
>> Other than that, quirks esp. for FPGA sounds so wrong. Why in the
>> first place not to make non-broken hardware?!
>
> I may have used the term 'quirk' incorrectly since they were placed in
> dwc3_pci_quirk(). There's no quirk for FPGA, but there are some initial
> setup properties for FPGA. To be specific, these properties:
> PROPERTY_ENTRY_BOOL("snps,usb3_lpm_capable"),
> PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
> PROPERTY_ENTRY_BOOL("snps,dis_enblslpm_quirk"),
> PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
OK.
--
With Best Regards,
Andy Shevchenko
--
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/7] usb: dwc3: pci: Store device properties dynamically
On Tue, Feb 20, 2018 at 11:12 PM, Thinh Nguyen <thinh.ngu...@synopsys.com> wrote: > On 2/17/2018 7:29 AM, Andy Shevchenko wrote: >> On Fri, Feb 16, 2018 at 11:55 PM, Thinh Nguyen >> <thinh.ngu...@synopsys.com> wrote: >>> Add the ability to add device properties dynamically. Currently, device >>> properties are added to platform device using >>> platform_device_add_properties(). However, this function does not allow >>> adding properties incrementally. It is useful to have this ability when >>> the driver needs to set common device properties across different HW >> >> I'm not sure it's useful anyhow. >> >>> or >>> if IP and FPGA validation test different configurations for different >>> HW. >> >> Shouldn't be a separate stuff for FPGA exclusively? > > Can you clarify/expand your question? FPGA is the one which might have different properties at run time for the same device. So, why do we care on driver / generic level of it? Shouldn't be FPGA manager take care of it (via DT overlays, for example)? >>> To address this issue, update dwc3_pci to store device properties to >>> an array and dynamically manage them here. >>> >>> Introduce two new functions to do so: >>> * dwc3_pci_add_one_property() - this function adds one property to >>> dwc->properties array and increase its size as needed >>> * dwc3_pci_add_properties() - this function takes a null terminated >>> array of device properties and add them to dwc->properties >> >> So, why you can't use ACPI / DT here? >> > > dwc3_pci_add_properties() is a convenient function that takes statically > allocated array of (quirks) properties for different HW and store them > to dwc->properties. The idea is to add more properties on top of these > required quirks. Yes, I understand that. What's wrong with DT? The built-in device properties have the same nature as usual properties for DT. Whenever driver calls device_property_read_uXX() or alike it would check all property provides for asked one. Other than that, quirks esp. for FPGA sounds so wrong. Why in the first place not to make non-broken hardware?! -- With Best Regards, Andy Shevchenko -- 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 v4 5/6] extcon: add possibility to get extcon device by OF node
On Wed, Feb 21, 2018 at 10:55 AM, Andrzej Hajda <a.ha...@samsung.com> wrote:
> Since extcon property is not allowed in DT, extcon subsystem requires
> another way to get extcon device. Lets try the simplest approach - get
> edev by of_node.
> +/*
> + * extcon_get_edev_by_of_node - Get the extcon device from devicetree.
> + * @node : OF node identyfying edev
> + *
> + * Return the pointer of extcon device if success or ERR_PTR(err) if fail.
> + */
> +struct extcon_dev *extcon_get_edev_by_of_node(struct device_node *node)
First of all, the all other similar cases use "_by_node" in the name.
Second, it's not _get_, it's _find_.
> +{
> + struct extcon_dev *edev;
> +
> + mutex_lock(_dev_list_lock);
> + list_for_each_entry(edev, _dev_list, entry)
> + if (edev->dev.parent && edev->dev.parent->of_node == node)
> + goto out;
> + edev = ERR_PTR(-EPROBE_DEFER);
> +out:
> + mutex_unlock(_dev_list_lock);
> +
> + return edev;
Can't it be done using bus_find_device()?
> +}
See good example in i2c-core-of.c
of_find_i2c_adapter_by_node()
of_get_i2c_adapter_by_node()
--
With Best Regards,
Andy Shevchenko
--
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: dwc3: debugfs: Re-use DEFINE_SHOW_ATTRIBUTE() macro
On Thu, 2018-02-15 at 15:12 +0200, Felipe Balbi wrote:
> Andy Shevchenko <andriy.shevche...@linux.intel.com> writes:
>
> > On Thu, 2018-02-15 at 13:16 +0200, Felipe Balbi wrote:
> > > ...instead of open coding file operations followed by custom
> > > ->open()
> > > callbacks per each attribute.
> > >
> >
> > Reviewed-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
> >
> > Though one comment below.
> >
> > > Signed-off-by: Felipe Balbi <felipe.ba...@linux.intel.com>
> > > static void dwc3_debugfs_create_endpoint_file(struct dwc3_ep
> > > *dep,
> > > + struct dentry *parent, const char *const name,
> > > + const struct file_operations *const fops)
> > > {
> > > + (void) debugfs_create_file(name, S_IRUGO, parent, dep,
> > > fops);
> > > }
> >
> > At this point why do you need one line function anymore?
>
> fair enough, here you go:
Feel free to add
Reviewed-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
> 8<--
>
> From 32ea5fda7654e7c081fff161544da73d53bf8fac Mon Sep 17 00:00:00 2001
> From: Felipe Balbi <felipe.ba...@linux.intel.com>
> Date: Thu, 15 Feb 2018 13:03:38 +0200
> Subject: [PATCH] usb: dwc3: debugfs: Re-use DEFINE_SHOW_ATTRIBUTE()
> macro
>
> ...instead of open coding file operations followed by custom ->open()
> callbacks per each attribute.
>
> Signed-off-by: Felipe Balbi <felipe.ba...@linux.intel.com>
> ---
> drivers/usb/dwc3/debugfs.c | 79 ++---
> -
> 1 file changed, 30 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
> index 00e65530c81e..c4c0dcb3f589 100644
> --- a/drivers/usb/dwc3/debugfs.c
> +++ b/drivers/usb/dwc3/debugfs.c
> @@ -487,8 +487,8 @@ static const struct file_operations
> dwc3_link_state_fops = {
> };
>
> struct dwc3_ep_file_map {
> - char name[25];
> - int (*show)(struct seq_file *s, void *unused);
> + const char name[25];
> + const struct file_operations *const fops;
> };
>
> static int dwc3_tx_fifo_queue_show(struct seq_file *s, void *unused)
> @@ -596,7 +596,7 @@ static int dwc3_event_queue_show(struct seq_file
> *s, void *unused)
> return 0;
> }
>
> -static int dwc3_ep_transfer_type_show(struct seq_file *s, void
> *unused)
> +static int dwc3_transfer_type_show(struct seq_file *s, void *unused)
> {
> struct dwc3_ep *dep = s->private;
> struct dwc3 *dwc = dep->dwc;
> @@ -632,7 +632,7 @@ static int dwc3_ep_transfer_type_show(struct
> seq_file *s, void *unused)
> return 0;
> }
>
> -static int dwc3_ep_trb_ring_show(struct seq_file *s, void *unused)
> +static int dwc3_trb_ring_show(struct seq_file *s, void *unused)
> {
> struct dwc3_ep *dep = s->private;
> struct dwc3 *dwc = dep->dwc;
> @@ -670,58 +670,39 @@ static int dwc3_ep_trb_ring_show(struct seq_file
> *s, void *unused)
> return 0;
> }
>
> -static struct dwc3_ep_file_map map[] = {
> - { "tx_fifo_queue", dwc3_tx_fifo_queue_show, },
> - { "rx_fifo_queue", dwc3_rx_fifo_queue_show, },
> - { "tx_request_queue", dwc3_tx_request_queue_show, },
> - { "rx_request_queue", dwc3_rx_request_queue_show, },
> - { "rx_info_queue", dwc3_rx_info_queue_show, },
> - { "descriptor_fetch_queue", dwc3_descriptor_fetch_queue_show,
> },
> - { "event_queue", dwc3_event_queue_show, },
> - { "transfer_type", dwc3_ep_transfer_type_show, },
> - { "trb_ring", dwc3_ep_trb_ring_show, },
> +DEFINE_SHOW_ATTRIBUTE(dwc3_tx_fifo_queue);
> +DEFINE_SHOW_ATTRIBUTE(dwc3_rx_fifo_queue);
> +DEFINE_SHOW_ATTRIBUTE(dwc3_tx_request_queue);
> +DEFINE_SHOW_ATTRIBUTE(dwc3_rx_request_queue);
> +DEFINE_SHOW_ATTRIBUTE(dwc3_rx_info_queue);
> +DEFINE_SHOW_ATTRIBUTE(dwc3_descriptor_fetch_queue);
> +DEFINE_SHOW_ATTRIBUTE(dwc3_event_queue);
> +DEFINE_SHOW_ATTRIBUTE(dwc3_transfer_type);
> +DEFINE_SHOW_ATTRIBUTE(dwc3_trb_ring);
> +
> +static const struct dwc3_ep_file_map dwc3_ep_file_map[] = {
> + { "tx_fifo_queue", _tx_fifo_queue_fops, },
> + { "rx_fifo_queue", _rx_fifo_queue_fops, },
> + { "tx_request_queue", _tx_request_queue_fops, },
> + { "rx_request_queue", _rx_request_queue_fops, },
> + { "rx_info_queue", _rx_info_queue_fops, },
> + { "descriptor_fetch
Re: [PATCH 0/7] usb: dwc3: pci: Make device properties customizable
On Fri, Feb 16, 2018 at 11:55 PM, Thinh Nguyen
<thinh.ngu...@synopsys.com> wrote:
> Add the ability for PCI platforms to customize device properties via
> debugfs attributes. For IP and FPGA validation purposes, this feature is
> useful to test against various HW configurations that are normally
> statically configured in a real HW. It allows changing of the device
> properties while using the same PID and VID without recompiling the
> driver with a different set of properties every time.
>
> Overview of this feature:
> * dwc3_pci_probe() will not immediately create and add platform device
>(That is, nothing will happen when dwc3-pci module is first loaded)
> * The user initiates platform device creation and addition to device
>hierarchy by writing to 'start' debugfs attribute
> * dwc3-pci creates debugfs attributes for device properties
> * The user can set these properties via those attributes
> * dwc3-pci stores the properties that the user set (they are added to
>platform device when user initiates 'start')
> * Device properties cannot be changed after the platform device has
>been added to device hierarchy
>
> This feature is not enabled by default, the user must enable
> CONFIG_USB_DWC3_PCI_DEBUGFS in order to use this feature. Currently
> there is only one property -- maximum_speed.
I dunno if Felipe likes the idea, but I would rather convert PCI glue
driver from being parent device to being the device (by using dwc3
platform driver as a library).
I also don't know if it's possible with regard to multiple quirks the
driver might have.
> Thinh Nguyen (7):
> usb: dwc3: pci: Properly cleanup resource
> usb: dwc3: pci: Set an easily recognizable device name
> usb: dwc3: pci: Store device properties dynamically
> usb: dwc3: pci: Move platform creation from probe function
> usb: dwc3: Rename dwc3-pci.c to pci.c
> usb: dwc3: pci: Create header file
> usb: dwc3: pci: Add debugfs for device properties
>
> drivers/usb/dwc3/Kconfig | 11 ++
> drivers/usb/dwc3/Makefile | 5 +
> drivers/usb/dwc3/dwc3-pci.h| 72 +++
> drivers/usb/dwc3/pci-debugfs.c | 144 ++
> drivers/usb/dwc3/{dwc3-pci.c => pci.c} | 214
> -
> 5 files changed, 392 insertions(+), 54 deletions(-)
> create mode 100644 drivers/usb/dwc3/dwc3-pci.h
> create mode 100644 drivers/usb/dwc3/pci-debugfs.c
> rename drivers/usb/dwc3/{dwc3-pci.c => pci.c} (77%)
>
> --
> 2.11.0
>
> --
> 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
--
With Best Regards,
Andy Shevchenko
--
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/7] usb: dwc3: pci: Store device properties dynamically
On Fri, Feb 16, 2018 at 11:55 PM, Thinh Nguyen <thinh.ngu...@synopsys.com> wrote: > Add the ability to add device properties dynamically. Currently, device > properties are added to platform device using > platform_device_add_properties(). However, this function does not allow > adding properties incrementally. It is useful to have this ability when > the driver needs to set common device properties across different HW I'm not sure it's useful anyhow. > or > if IP and FPGA validation test different configurations for different > HW. Shouldn't be a separate stuff for FPGA exclusively? > To address this issue, update dwc3_pci to store device properties to > an array and dynamically manage them here. > > Introduce two new functions to do so: > * dwc3_pci_add_one_property() - this function adds one property to >dwc->properties array and increase its size as needed > * dwc3_pci_add_properties() - this function takes a null terminated >array of device properties and add them to dwc->properties So, why you can't use ACPI / DT here? -- With Best Regards, Andy Shevchenko -- 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 2/7] usb: dwc3: pci: Set an easily recognizable device name
On Fri, Feb 16, 2018 at 11:55 PM, Thinh Nguyen <thinh.ngu...@synopsys.com> wrote: > Set the device name for PCI glue layer to "dwc3-pci.BB:SS.FF" where BB > is PCI bus number, SS is PCI slot, and FF is PCI function number. Isn't pci_name(pci) actually returns you a BDF notation of the device? > --- a/drivers/usb/dwc3/dwc3-pci.c > +++ b/drivers/usb/dwc3/dwc3-pci.c > @@ -229,6 +229,9 @@ static int dwc3_pci_probe(struct pci_dev *pci, > dwc->dwc3->dev.parent = dev; > ACPI_COMPANION_SET(>dwc3->dev, ACPI_COMPANION(dev)); > > + dev_set_name(dev, "dwc3-pci.%02x:%02x.%d", pci->bus->number, > + PCI_SLOT(pci->devfn), PCI_FUNC(pci->devfn)); > + -- With Best Regards, Andy Shevchenko -- 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 04/12] usb: common: Small class for USB role switches
On Fri, Feb 16, 2018 at 12:47 PM, Hans de Goede <hdego...@redhat.com> wrote:
> USB role switch is a device that can be used to choose the
> data role for USB connector. With dual-role capable USB
> controllers, the controller itself will be the switch, but
> on some platforms the USB host and device controllers are
> separate IPs and there is a mux between them and the
> connector. On those platforms the mux driver will need to
> register the switch.
>
> With USB Type-C connectors, the host-to-device relationship
> is negotiated over the Configuration Channel (CC). That
> means the USB Type-C drivers need to be in control of the
> role switch. The class provides a simple API for the USB
> Type-C drivers for the control.
>
> For other types of USB connectors (mainly microAB) the class
> provides user space control via sysfs attribute file that
> can be used to request role swapping from the switch.
> +static int __switch_match(struct device *dev, const void *name)
bool?
> +{
> + return !strcmp((const char *)name, dev_name(dev));
> +}
> +static ssize_t role_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct usb_role_switch *sw = to_role_switch(dev);
> + int ret;
> +
> + ret = sysfs_match_string(usb_roles, buf);
> + if (ret < 0) {
> + bool res;
> +
> + /* Extra check if the user wants to disable the switch */
> + ret = kstrtobool(buf, );
> + if (ret || res)
> + return -EINVAL;
> + }
> +
> + ret = usb_role_switch_set_role(sw, ret);
> + if (!ret)
> + ret = size;
> +
> + return ret;
Perhaps
ret = ...
if (ret)
return ret;
return size;
> +}
> +struct usb_role_switch *
> +usb_role_switch_register(struct device *parent,
> +const struct usb_role_switch_desc *desc)
> +{
> + struct usb_role_switch *sw;
> + int ret;
> +
> + if (!desc || !desc->set)
> + return ERR_PTR(-EINVAL);
> +
> + sw = kzalloc(sizeof(*sw), GFP_KERNEL);
> + if (!sw)
> + return ERR_PTR(-ENOMEM);
> + ret = device_register(>dev);
> + if (ret) {
> + put_device(>dev);
Memory leak?
> + return ERR_PTR(ret);
> + }
> +
> + /* TODO: Symlinks for the host port and the device controller. */
> +
> + return sw;
> +}
> +void usb_role_switch_unregister(struct usb_role_switch *sw)
> +{
> + if (sw && !IS_ERR(sw))
!IS_ERR_OR_NULL() ?
> + device_unregister(>dev);
> +}
--
With Best Regards,
Andy Shevchenko
--
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 03/12] usb: typec: API for controlling USB Type-C Multiplexers
On Fri, Feb 16, 2018 at 12:47 PM, Hans de Goede <hdego...@redhat.com> wrote: > USB Type-C connectors consist of various muxes and switches > that route the pins on the connector to the right locations. > The USB Type-C drivers need to be able to control the muxes, > as they are the ones that know things like the cable plug > orientation, and the current mode that was negotiated with > the partner. > > This introduces a small API for registering and controlling > cable plug orientation switches, and separate small API for > registering and controlling pin multiplexer/demultiplexer > switches that are needed with Accessory/Alternate Modes. > + sw = __device_find_connection(dev, "typec-switch", NULL, > + typec_switch_match); Perhaps one line? (even if it takes ~83 characters) -- With Best Regards, Andy Shevchenko -- 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 08/12] xhci: Add Intel extended cap / otg phy mux handling
On Fri, Feb 16, 2018 at 12:47 PM, Hans de Goede <hdego...@redhat.com> wrote:
> The xHCI controller on various Intel SoCs has an extended cap mmio-range
> which contains registers to control the muxing to the xHCI (host mode)
> or the dwc3 (device mode) and vbus-detection for the otg usb-phy.
>
> Having a role-sw driver included in the xhci code (under drivers/usb/host)
> is not desirable. So this commit adds a simple handler for this extended
> capability, which creates a platform device with the caps mmio region as
> resource, this allows us to write a separate platform role-sw driver for
> the role-switch.
>
> Note this commit adds a call to the new xhci_ext_cap_init() function
> to xhci_pci_probe(), it is added here because xhci_ext_cap_init() must
> be called only once. If in the future we also want to handle ext-caps
> on non pci xHCI HCDs from xhci_ext_cap_init() a call to it should also
> be added to other bus probe paths.
SPDX?
> +/*
> + * XHCI extended capability handling
> + *
> + * Copyright (c) 2017 Hans de Goede <hdego...@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> + pdev = platform_device_alloc("intel_xhci_usb_sw",
> PLATFORM_DEVID_NONE);
Perhaps,
#define USB_SW_DRV_NAME "..."
> + if (!pdev) {
> + xhci_err(xhci, "couldn't allocate intel_xhci_usb_sw pdev\n");
...and re-use it everywhere here.
pdev -> platform device.
> + return -ENOMEM;
> + }
> +
> + res.start = hcd->rsrc_start + cap_offset;
> + res.end = res.start + 0x3ff;
Is this magic always the same? Where its value comes from?
At least define with comment.
> +int xhci_ext_cap_init(struct xhci_hcd *xhci)
> +{
> + void __iomem *base = >cap_regs->hc_capbase;
> + u32 cap_offset, val;
> + int ret;
> +
> + cap_offset = xhci_find_next_ext_cap(base, 0, 0);
> +
> + while (cap_offset) {
> + val = readl(base + cap_offset);
> +
> + switch (XHCI_EXT_CAPS_ID(val)) {
> + case XHCI_EXT_CAPS_VENDOR_INTEL:
> + if (xhci->quirks & XHCI_INTEL_USB_ROLE_SW) {
> + ret = xhci_create_intel_xhci_sw_pdev(
> + xhci, cap_offset);
Can we leave xhci on previous line?
> + if (ret)
> + return ret;
> + }
> + break;
> + }
> + cap_offset = xhci_find_next_ext_cap(base, cap_offset, 0);
> + }
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
--
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 09/12] usb: roles: Add Intel XHCI USB role switch driver
On Fri, Feb 16, 2018 at 12:47 PM, Hans de Goede <hdego...@redhat.com> wrote:
> Various Intel SoCs (Cherry Trail, Broxton and others) have an internal USB
> role switch for swiching the OTG USB data lines between the xHCI host
> controller and the dwc3 gadget controller.
>
> Note on some Cherry Trail systems there is ACPI/AML code listening to
> edge interrupts on the id-pin (through an _AIE ACPI method) and switching
> the role between ROLE_HOST and ROLE_NONE based on the id-pin. Note it does
> not set the role to ROLE_DEVICE, because device-mode is usually not used
> under Windows.
>
> The presence of AML code which modifies the cfg0 reg (on some systems)
> means that we our read/write/modify of cfg0 may race with the AML code
> doing the same to avoid this we take the global ACPI lock while doing
> the read/write/modify.
> +/* register definition */
> +#define DUAL_ROLE_CFG0 0x68
> +#define SW_VBUS_VALID (1 << 24)
> +#define SW_IDPIN_EN(1 << 21)
> +#define SW_IDPIN (1 << 20)
> +
> +#define DUAL_ROLE_CFG1 0x6c
> +#define HOST_MODE (1 << 29)
Does it make sense to use BIT() macro above?
> +struct intel_xhci_acpi_match {
> + const char *hid;
> + int hrv;
> +};
Consider to unify with struct acpi_ac_bl.
> +static const struct intel_xhci_acpi_match allow_userspace_ctrl_ids[] = {
> + { "INT33F4", 3 }, /* X-Powers AXP288 PMIC */
> +};
> +
> +static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> +{
> + struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
> + unsigned long timeout;
> + acpi_status status;
> + u32 glk = -1U;
I prefer to see consistency and moreover less confusing set, like
~0U
> + u32 val;
> +
> + /*
> +* On many CHT devices ACPI event (_AEI) handlers read / modify /
> +* write the cfg0 register, just like we do. Take the ACPI lock
> +* to avoid us racing with the AML code.
> +*/
> + status = acpi_acquire_global_lock(ACPI_WAIT_FOREVER, );
FOREVER?!
Wouldn't be slightly long under certain circumstances?
> + if (ACPI_FAILURE(status) && status != AE_NOT_CONFIGURED) {
> + dev_err(dev, "Error could not acquire lock\n");
> + return -EIO;
> + }
> + acpi_release_global_lock(glk);
> + /* Polling on CFG1 register to confirm mode switch.*/
> + do {
> + val = readl(data->base + DUAL_ROLE_CFG1);
> + if (!!(val & HOST_MODE) == (role == USB_ROLE_HOST))
I would prefer ^ instead of first ==, but it's up to you.
> + return 0;
> +
> + /* Interval for polling is set to about 5 - 10 ms */
> + usleep_range(5000, 1);
> + } while (time_before(jiffies, timeout));
> +
> + dev_warn(dev, "Timeout waiting for role-switch\n");
> + return -ETIMEDOUT;
> +}
> +static int intel_xhci_usb_probe(struct platform_device *pdev)
> +{
> + struct device *dev = >dev;
> + struct intel_xhci_usb_data *data;
> + struct resource *res;
> + resource_size_t size;
> + int i, ret;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + size = (res->end + 1) - res->start;
resource_size()
> + data->base = devm_ioremap_nocache(dev, res->start, size);
So, what's wrong with devm_ioremap_resource() ?
...which also prints an error message.
> + if (IS_ERR(data->base)) {
> + ret = PTR_ERR(data->base);
> + dev_err(dev, "Error iomaping registers: %d\n", ret);
At least printing return code is useless. Driver core does this.
> + return ret;
> + }
> +
> + data->role_sw = usb_role_switch_register(dev, _desc);
> + if (IS_ERR(data->role_sw)) {
> + ret = PTR_ERR(data->role_sw);
> + dev_err(dev, "Error registering role-switch: %d\n", ret);
Ditto.
> + return ret;
> + }
> +
> + return 0;
> +}
> +static const struct platform_device_id intel_xhci_usb_table[] = {
> + { .name = DRV_NAME },
> + {},
No comma, please.
> +};
--
With Best Regards,
Andy Shevchenko
--
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 00/12] USB Type-C device-connection, mux and switch support
On Fri, Feb 16, 2018 at 12:47 PM, Hans de Goede <hdego...@redhat.com> wrote: While I'm going through patches, one notice to all of them: be consistent with xHCI abbreviation. Also check others, like vBus (or whatever is standard de facto to show it in comments and to the user in messages). -- With Best Regards, Andy Shevchenko -- 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 10/12] usb: typec: driver for Pericom PI3USB30532 Type-C cross switch
On Fri, Feb 16, 2018 at 12:47 PM, Hans de Goede <hdego...@redhat.com> wrote:
> Add a driver for the Pericom PI3USB30532 Type-C cross switch /
> mux chip found on some devices with a Type-C port.
> +static int pi3usb30532_set_conf(struct pi3usb30532 *pi, u8 new_conf)
> +{
> + int ret = 0;
> +
> + if (pi->conf == new_conf)
> + return 0;
> +
> + ret = i2c_smbus_write_byte_data(pi->client, PI3USB30532_CONF,
> new_conf);
> + if (ret == 0)
> + pi->conf = new_conf;
> + else
> + dev_err(>client->dev, "Error writing conf: %d\n", ret);
Usual pattern, please.
if (ret) {
...
return ret;
}
return 0;
> +
> + return ret;
> +}
--
With Best Regards,
Andy Shevchenko
--
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 12/12] extcon: axp288: Set USB role where necessary
On Fri, Feb 16, 2018 at 12:47 PM, Hans de Goede <hdego...@redhat.com> wrote:
> The AXP288 BC1.2 charger detection / extcon code may seem like a strange
> place to add code to control the USB role-switch on devices with an AXP288,
> but there are 2 reasons to do this inside the axp288 extcon code:
>
> 1) On many devices the USB role is controlled by ACPI AML code, but the AML
>code only switches between the host and none roles, because of Windows
>not really using device mode. To make device mode work we need to toggle
>between the none/device roles based on VBus presence, and the axp288
>extcon gets interrupts on VBus insertion / removal.
>
> 2) In order for our BC1.2 charger detection to work properly the role
>mux must be properly set to device mode before we do the detection.
>
> Also note the Kconfig help-text / obsolete depends on USB_PHY which are
> remnants from older never upstreamed code also controlling the mux from
> the axp288 extcon code.
>
> This commit also adds code to get notifications from the INT3496 extcon
> device, which is used on some devices to notify the kernel about id-pin
> changes instead of them being handled through AML code.
>
> This fixes:
> -Device mode not working on most CHT devices with an AXP288
> -Host mode not working on devices with an INT3496 ACPI device
> -Charger-type misdetection (always SDP) on devices with an INT3496 when the
> USB role (always) gets initialized as host
>
> Signed-off-by: Hans de Goede <hdego...@redhat.com>
> config EXTCON_AXP288
> tristate "X-Power AXP288 EXTCON support"
> - depends on MFD_AXP20X && USB_PHY
> + depends on MFD_AXP20X && USB_SUPPORT
> + select USB_ROLE_SWITCH
Is it supposed to work outside of x86 world?..
> +#include
> +#include
...if yes, this should go under CONFIG_X86 along with accompanying parts.
...if no, put corresponding dependency to Kconfig.
> + if (info->role_sw) {
> + ret = devm_add_action_or_reset(dev, axp288_put_role_sw, info);
> + if (ret)
> + return ret;
> +
> + if (acpi_dev_present("INT3496", NULL, -1)) {
> + info->id_extcon = extcon_get_extcon_dev("INT3496:00");
Please use instance found by acpi_dev_present(). Okay, actually new
helper is here:
acpi_dev_get_first_match_name().
> + if (!info->id_extcon)
> + return -EPROBE_DEFER;
> +
> + dev_info(dev, "controlling USB role\n");
> + } else {
> + dev_info(dev, "controlling USB role based on vbus
> presence\n");
> + }
> + }
> +
--
With Best Regards,
Andy Shevchenko
--
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 11/12] platform/x86: intel_cht_int33fe: Add device connections for the Type-C port
On Fri, Feb 16, 2018 at 12:47 PM, Hans de Goede <hdego...@redhat.com> wrote:
> We need to add device-connections for the TYPE-C mux/switch and usb-role
> code to be able to find the PI3USB30532 Type-C cross-switch and the
> device/host switch integrated in the CHT SoC.
>
Acked-by: Andy Shevchenko <andy.shevche...@gmail.com>
> Signed-off-by: Hans de Goede <hdego...@redhat.com>
> ---
> drivers/platform/x86/intel_cht_int33fe.c | 25 +
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/platform/x86/intel_cht_int33fe.c
> b/drivers/platform/x86/intel_cht_int33fe.c
> index 380ef7ec094f..a3f8674f14da 100644
> --- a/drivers/platform/x86/intel_cht_int33fe.c
> +++ b/drivers/platform/x86/intel_cht_int33fe.c
> @@ -21,6 +21,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -33,6 +34,8 @@ struct cht_int33fe_data {
> struct i2c_client *max17047;
> struct i2c_client *fusb302;
> struct i2c_client *pi3usb30532;
> + /* Contain a list-head must be per device */
> + struct devcon connections[3];
> };
>
> /*
> @@ -172,6 +175,20 @@ static int cht_int33fe_probe(struct i2c_client *client)
> return -EPROBE_DEFER; /* Wait for i2c-adapter to load
> */
> }
>
> + data->connections[0].endpoint[0] = "i2c-fusb302";
> + data->connections[0].endpoint[1] = "i2c-pi3usb30532";
> + data->connections[0].id = "typec-switch";
> + data->connections[1].endpoint[0] = "i2c-fusb302";
> + data->connections[1].endpoint[1] = "i2c-pi3usb30532";
> + data->connections[1].id = "typec-mux";
> + data->connections[2].endpoint[0] = "i2c-fusb302";
> + data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch";
> + data->connections[2].id = "usb-role-switch";
> +
> + add_device_connection(>connections[0]);
> + add_device_connection(>connections[1]);
> + add_device_connection(>connections[2]);
> +
> memset(_info, 0, sizeof(board_info));
> strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
> board_info.dev_name = "fusb302";
> @@ -201,6 +218,10 @@ static int cht_int33fe_probe(struct i2c_client *client)
> if (data->max17047)
> i2c_unregister_device(data->max17047);
>
> + remove_device_connection(>connections[2]);
> + remove_device_connection(>connections[1]);
> + remove_device_connection(>connections[0]);
> +
> return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
> }
>
> @@ -213,6 +234,10 @@ static int cht_int33fe_remove(struct i2c_client *i2c)
> if (data->max17047)
> i2c_unregister_device(data->max17047);
>
> + remove_device_connection(>connections[2]);
> + remove_device_connection(>connections[1]);
> + remove_device_connection(>connections[0]);
> +
> return 0;
> }
>
> --
> 2.14.3
>
--
With Best Regards,
Andy Shevchenko
--
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 02/12] usb: typec: Start using ERR_PTR
On Fri, Feb 16, 2018 at 12:47 PM, Hans de Goede <hdego...@redhat.com> wrote:
> From: Heikki Krogerus <heikki.kroge...@linux.intel.com>
>
> In order to allow the USB Type-C Class driver take care of
> things like muxes and other possible dependencies for the
> port drivers, returning ERR_PTR instead of NULL from the
> registration functions in case of failure.
>
> The reason for taking over control of the muxes for example
> is because handling them in the port drivers would be just
> boilerplate.
> void typec_unregister_altmode(struct typec_altmode *alt)
> {
> - if (alt)
> - device_unregister(>dev);
> + device_unregister(>dev);
> }
But it's a pattern to guarantee that unregister type of functions are
NULL (or ERR_PTR) aware.
> void typec_unregister_partner(struct typec_partner *partner)
> {
> - if (partner)
> - device_unregister(>dev);
> + device_unregister(>dev);
> }
Ditto.
--
With Best Regards,
Andy Shevchenko
--
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 01/12] drivers: base: Unified device connection lookup
On Fri, Feb 16, 2018 at 12:47 PM, Hans de Goede <hdego...@redhat.com> wrote:
> From: Heikki Krogerus <heikki.kroge...@linux.intel.com>
>
> Several frameworks - clk, gpio, phy, pmw, etc. - maintain
> lookup tables for describing connections and provide custom
> API for handling them. This introduces a single generic
> lookup table and API for the connections.
>
> The motivation for this commit is centralizing the
> connection lookup, but the goal is to ultimately extract the
> connection descriptions also from firmware by using the
> fwnode_graph_* functions and other mechanisms that are
> available.
> +void *__device_find_connection(struct device *dev, const char *con_id,
> + void *data,
> + void *(*match)(struct devcon *con, int ep,
> + void *data))
Perhaps swap them, since data is dependent parameter to match.
And put match function on one line disregard 80 character limit?
> +/**
> + * struct devcon - Device Connection Descriptor
> + * @endpoint: The names of the two devices connected together
> + * @id: Unique identifier for the connection
> + */
> +struct devcon {
> + const char *endpoint[2];
> + const char *id;
> + struct list_headlist;
> +};
> +
> +void *__device_find_connection(struct device *dev, const char *con_id,
> + void *data,
> + void *(*match)(struct devcon *con, int ep,
> + void *data));
Ditto.
> +
> +struct device *device_find_connection(struct device *dev, const char
> *con_id);
> +
> +#define DEVCON(_ep0, _ep1, _id){ { _ep0, _ep1 }, _id, }
Please use (struct devcon) here to make it possible to do like
struct devcon foo;
foo = DEVCON(...);
--
With Best Regards,
Andy Shevchenko
--
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: dwc3: debugfs: Re-use DEFINE_SHOW_ATTRIBUTE() macro
On Thu, 2018-02-15 at 13:16 +0200, Felipe Balbi wrote:
> ...instead of open coding file operations followed by custom ->open()
> callbacks per each attribute.
>
Reviewed-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
Though one comment below.
> Signed-off-by: Felipe Balbi <felipe.ba...@linux.intel.com>
> static void dwc3_debugfs_create_endpoint_file(struct dwc3_ep *dep,
> + struct dentry *parent, const char *const name,
> + const struct file_operations *const fops)
> {
> + (void) debugfs_create_file(name, S_IRUGO, parent, dep, fops);
> }
At this point why do you need one line function anymore?
> + dwc3_debugfs_create_endpoint_file(dep, parent,
> + name, fops);
--
Andy Shevchenko <andriy.shevche...@linux.intel.com>
Intel Finland Oy
--
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 v1 09/14] USB: host: imx21: Re-use DEFINE_SHOW_ATTRIBUTE() macro
...instead of open coding file operations followed by custom ->open()
callbacks per each attribute.
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
drivers/usb/host/imx21-dbg.c | 65
1 file changed, 5 insertions(+), 60 deletions(-)
diff --git a/drivers/usb/host/imx21-dbg.c b/drivers/usb/host/imx21-dbg.c
index b964f9a51d87..a213ed6f07b5 100644
--- a/drivers/usb/host/imx21-dbg.c
+++ b/drivers/usb/host/imx21-dbg.c
@@ -245,6 +245,7 @@ static int debug_status_show(struct seq_file *s, void *v)
return 0;
}
+DEFINE_SHOW_ATTRIBUTE(debug_status);
static int debug_dmem_show(struct seq_file *s, void *v)
{
@@ -266,6 +267,7 @@ static int debug_dmem_show(struct seq_file *s, void *v)
return 0;
}
+DEFINE_SHOW_ATTRIBUTE(debug_dmem);
static int debug_etd_show(struct seq_file *s, void *v)
{
@@ -334,6 +336,7 @@ static int debug_etd_show(struct seq_file *s, void *v)
return 0;
}
+DEFINE_SHOW_ATTRIBUTE(debug_etd);
static void debug_statistics_show_one(struct seq_file *s,
const char *name, struct debug_stats *stats)
@@ -368,6 +371,7 @@ static int debug_statistics_show(struct seq_file *s, void
*v)
return 0;
}
+DEFINE_SHOW_ATTRIBUTE(debug_statistics);
static void debug_isoc_show_one(struct seq_file *s,
const char *name, int index,struct debug_isoc_trace *trace)
@@ -409,66 +413,7 @@ static int debug_isoc_show(struct seq_file *s, void *v)
return 0;
}
-
-static int debug_status_open(struct inode *inode, struct file *file)
-{
- return single_open(file, debug_status_show, inode->i_private);
-}
-
-static int debug_dmem_open(struct inode *inode, struct file *file)
-{
- return single_open(file, debug_dmem_show, inode->i_private);
-}
-
-static int debug_etd_open(struct inode *inode, struct file *file)
-{
- return single_open(file, debug_etd_show, inode->i_private);
-}
-
-static int debug_statistics_open(struct inode *inode, struct file *file)
-{
- return single_open(file, debug_statistics_show, inode->i_private);
-}
-
-static int debug_isoc_open(struct inode *inode, struct file *file)
-{
- return single_open(file, debug_isoc_show, inode->i_private);
-}
-
-static const struct file_operations debug_status_fops = {
- .open = debug_status_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
-
-static const struct file_operations debug_dmem_fops = {
- .open = debug_dmem_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
-
-static const struct file_operations debug_etd_fops = {
- .open = debug_etd_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
-
-static const struct file_operations debug_statistics_fops = {
- .open = debug_statistics_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
-
-static const struct file_operations debug_isoc_fops = {
- .open = debug_isoc_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(debug_isoc);
static void create_debug_files(struct imx21 *imx21)
{
--
2.15.1
--
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 v1 14/14] uwb: Re-use DEFINE_SHOW_ATTRIBUTE() macro
...instead of open coding file operations followed by custom ->open()
callbacks per each attribute.
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
drivers/uwb/uwb-debug.c | 32
1 file changed, 4 insertions(+), 28 deletions(-)
diff --git a/drivers/uwb/uwb-debug.c b/drivers/uwb/uwb-debug.c
index 991374b13571..f1622bae13be 100644
--- a/drivers/uwb/uwb-debug.c
+++ b/drivers/uwb/uwb-debug.c
@@ -206,7 +206,7 @@ static const struct file_operations command_fops = {
.owner = THIS_MODULE,
};
-static int reservations_print(struct seq_file *s, void *p)
+static int reservations_show(struct seq_file *s, void *p)
{
struct uwb_rc *rc = s->private;
struct uwb_rsv *rsv;
@@ -240,21 +240,9 @@ static int reservations_print(struct seq_file *s, void *p)
return 0;
}
+DEFINE_SHOW_ATTRIBUTE(reservations);
-static int reservations_open(struct inode *inode, struct file *file)
-{
- return single_open(file, reservations_print, inode->i_private);
-}
-
-static const struct file_operations reservations_fops = {
- .open= reservations_open,
- .read= seq_read,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
-
-static int drp_avail_print(struct seq_file *s, void *p)
+static int drp_avail_show(struct seq_file *s, void *p)
{
struct uwb_rc *rc = s->private;
@@ -264,19 +252,7 @@ static int drp_avail_print(struct seq_file *s, void *p)
return 0;
}
-
-static int drp_avail_open(struct inode *inode, struct file *file)
-{
- return single_open(file, drp_avail_print, inode->i_private);
-}
-
-static const struct file_operations drp_avail_fops = {
- .open= drp_avail_open,
- .read= seq_read,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_ATTRIBUTE(drp_avail);
static void uwb_dbg_channel_changed(struct uwb_pal *pal, int channel)
{
--
2.15.1
--
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 v1 02/14] USB: dwc2: Re-use DEFINE_SHOW_ATTRIBUTE() macro
...instead of open coding file operations followed by custom ->open()
callbacks per each attribute.
Cc: John Youn <johny...@synopsys.com>
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
drivers/usb/dwc2/debugfs.c | 84 --
1 file changed, 6 insertions(+), 78 deletions(-)
diff --git a/drivers/usb/dwc2/debugfs.c b/drivers/usb/dwc2/debugfs.c
index f4650a88be78..5e0d7f2bd2af 100644
--- a/drivers/usb/dwc2/debugfs.c
+++ b/drivers/usb/dwc2/debugfs.c
@@ -170,19 +170,7 @@ static int state_show(struct seq_file *seq, void *v)
return 0;
}
-
-static int state_open(struct inode *inode, struct file *file)
-{
- return single_open(file, state_show, inode->i_private);
-}
-
-static const struct file_operations state_fops = {
- .owner = THIS_MODULE,
- .open = state_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(state);
/**
* fifo_show - debugfs: show the fifo information
@@ -219,19 +207,7 @@ static int fifo_show(struct seq_file *seq, void *v)
return 0;
}
-
-static int fifo_open(struct inode *inode, struct file *file)
-{
- return single_open(file, fifo_show, inode->i_private);
-}
-
-static const struct file_operations fifo_fops = {
- .owner = THIS_MODULE,
- .open = fifo_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(fifo);
static const char *decode_direction(int is_in)
{
@@ -303,19 +279,7 @@ static int ep_show(struct seq_file *seq, void *v)
return 0;
}
-
-static int ep_open(struct inode *inode, struct file *file)
-{
- return single_open(file, ep_show, inode->i_private);
-}
-
-static const struct file_operations ep_fops = {
- .owner = THIS_MODULE,
- .open = ep_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(ep);
/**
* dwc2_hsotg_create_debug - create debugfs directory and files
@@ -770,19 +734,7 @@ static int params_show(struct seq_file *seq, void *v)
return 0;
}
-
-static int params_open(struct inode *inode, struct file *file)
-{
- return single_open(file, params_show, inode->i_private);
-}
-
-static const struct file_operations params_fops = {
- .owner = THIS_MODULE,
- .open = params_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(params);
static int hw_params_show(struct seq_file *seq, void *v)
{
@@ -817,19 +769,7 @@ static int hw_params_show(struct seq_file *seq, void *v)
return 0;
}
-
-static int hw_params_open(struct inode *inode, struct file *file)
-{
- return single_open(file, hw_params_show, inode->i_private);
-}
-
-static const struct file_operations hw_params_fops = {
- .owner = THIS_MODULE,
- .open = hw_params_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(hw_params);
static int dr_mode_show(struct seq_file *seq, void *v)
{
@@ -840,19 +780,7 @@ static int dr_mode_show(struct seq_file *seq, void *v)
seq_printf(seq, "%s\n", dr_mode);
return 0;
}
-
-static int dr_mode_open(struct inode *inode, struct file *file)
-{
- return single_open(file, dr_mode_show, inode->i_private);
-}
-
-static const struct file_operations dr_mode_fops = {
- .owner = THIS_MODULE,
- .open = dr_mode_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(dr_mode);
int dwc2_debugfs_init(struct dwc2_hsotg *hsotg)
{
--
2.15.1
--
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 v1 13/14] USB: typec: Re-use DEFINE_SHOW_ATTRIBUTE() macro
...instead of open coding file operations followed by custom ->open()
callbacks per each attribute.
Cc: Heikki Krogerus <heikki.kroge...@linux.intel.com>
Cc: Guenter Roeck <li...@roeck-us.net>
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
drivers/usb/typec/fusb302/fusb302.c | 17 +++--
drivers/usb/typec/tcpm.c| 17 +++--
2 files changed, 6 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/typec/fusb302/fusb302.c
b/drivers/usb/typec/fusb302/fusb302.c
index 9ce4756adad6..da179aaf789e 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -199,7 +199,7 @@ static void fusb302_log(struct fusb302_chip *chip, const
char *fmt, ...)
va_end(args);
}
-static int fusb302_seq_show(struct seq_file *s, void *v)
+static int fusb302_debug_show(struct seq_file *s, void *v)
{
struct fusb302_chip *chip = (struct fusb302_chip *)s->private;
int tail;
@@ -216,18 +216,7 @@ static int fusb302_seq_show(struct seq_file *s, void *v)
return 0;
}
-
-static int fusb302_debug_open(struct inode *inode, struct file *file)
-{
- return single_open(file, fusb302_seq_show, inode->i_private);
-}
-
-static const struct file_operations fusb302_debug_operations = {
- .open = fusb302_debug_open,
- .llseek = seq_lseek,
- .read = seq_read,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(fusb302_debug);
static struct dentry *rootdir;
@@ -242,7 +231,7 @@ static int fusb302_debugfs_init(struct fusb302_chip *chip)
chip->dentry = debugfs_create_file(dev_name(chip->dev),
S_IFREG | 0444, rootdir,
- chip, _debug_operations);
+ chip, _debug_fops);
return 0;
}
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index f4d563ee7690..a163ba55b061 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -506,7 +506,7 @@ static void tcpm_log_source_caps(struct tcpm_port *port)
}
}
-static int tcpm_seq_show(struct seq_file *s, void *v)
+static int tcpm_debug_show(struct seq_file *s, void *v)
{
struct tcpm_port *port = (struct tcpm_port *)s->private;
int tail;
@@ -523,18 +523,7 @@ static int tcpm_seq_show(struct seq_file *s, void *v)
return 0;
}
-
-static int tcpm_debug_open(struct inode *inode, struct file *file)
-{
- return single_open(file, tcpm_seq_show, inode->i_private);
-}
-
-static const struct file_operations tcpm_debug_operations = {
- .open = tcpm_debug_open,
- .llseek = seq_lseek,
- .read = seq_read,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(tcpm_debug);
static struct dentry *rootdir;
@@ -550,7 +539,7 @@ static int tcpm_debugfs_init(struct tcpm_port *port)
port->dentry = debugfs_create_file(dev_name(port->dev),
S_IFREG | 0444, rootdir,
- port, _debug_operations);
+ port, _debug_fops);
return 0;
}
--
2.15.1
--
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 v1 03/14] USB: musb: Re-use DEFINE_SHOW_ATTRIBUTE() macro
...instead of open coding file operations followed by custom ->open()
callbacks per each attribute.
Cc: Bin Liu <b-...@ti.com>
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
drivers/usb/musb/musb_debugfs.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/usb/musb/musb_debugfs.c b/drivers/usb/musb/musb_debugfs.c
index 7cf5a1bbdaff..025b2c8630df 100644
--- a/drivers/usb/musb/musb_debugfs.c
+++ b/drivers/usb/musb/musb_debugfs.c
@@ -112,11 +112,7 @@ static int musb_regdump_show(struct seq_file *s, void
*unused)
pm_runtime_put_autosuspend(musb->controller);
return 0;
}
-
-static int musb_regdump_open(struct inode *inode, struct file *file)
-{
- return single_open(file, musb_regdump_show, inode->i_private);
-}
+DEFINE_SHOW_ATTRIBUTE(musb_regdump);
static int musb_test_mode_show(struct seq_file *s, void *unused)
{
@@ -161,13 +157,6 @@ static int musb_test_mode_show(struct seq_file *s, void
*unused)
return 0;
}
-static const struct file_operations musb_regdump_fops = {
- .open = musb_regdump_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
-
static int musb_test_mode_open(struct inode *inode, struct file *file)
{
return single_open(file, musb_test_mode_show, inode->i_private);
--
2.15.1
--
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 v1 05/14] USB: gadget: gr: Re-use DEFINE_SHOW_ATTRIBUTE() macro
...instead of open coding file operations followed by custom ->open()
callbacks per each attribute.
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
drivers/usb/gadget/udc/gr_udc.c | 17 ++---
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/gadget/udc/gr_udc.c b/drivers/usb/gadget/udc/gr_udc.c
index b3fb1bbdb854..ca83c15d8ea4 100644
--- a/drivers/usb/gadget/udc/gr_udc.c
+++ b/drivers/usb/gadget/udc/gr_udc.c
@@ -179,8 +179,7 @@ static void gr_seq_ep_show(struct seq_file *seq, struct
gr_ep *ep)
seq_puts(seq, "\n");
}
-
-static int gr_seq_show(struct seq_file *seq, void *v)
+static int gr_dfs_show(struct seq_file *seq, void *v)
{
struct gr_udc *dev = seq->private;
u32 control = gr_read32(>regs->control);
@@ -203,19 +202,7 @@ static int gr_seq_show(struct seq_file *seq, void *v)
return 0;
}
-
-static int gr_dfs_open(struct inode *inode, struct file *file)
-{
- return single_open(file, gr_seq_show, inode->i_private);
-}
-
-static const struct file_operations gr_dfs_fops = {
- .owner = THIS_MODULE,
- .open = gr_dfs_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(gr_dfs);
static void gr_dfs_create(struct gr_udc *dev)
{
--
2.15.1
--
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 v1 04/14] USB: gadget: bcm63xx: Re-use DEFINE_SHOW_ATTRIBUTE() macro
...instead of open coding file operations followed by custom ->open()
callbacks per each attribute.
Cc: Kevin Cernekee <cerne...@gmail.com>
Cc: Florian Fainelli <f.faine...@gmail.com>
Cc: bcm-kernel-feedback-l...@broadcom.com
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
drivers/usb/gadget/udc/bcm63xx_udc.c | 33 -
1 file changed, 4 insertions(+), 29 deletions(-)
diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c
b/drivers/usb/gadget/udc/bcm63xx_udc.c
index 465ccd1104de..3a8df8601074 100644
--- a/drivers/usb/gadget/udc/bcm63xx_udc.c
+++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
@@ -2158,6 +2158,7 @@ static int bcm63xx_usbd_dbg_show(struct seq_file *s, void
*p)
return 0;
}
+DEFINE_SHOW_ATTRIBUTE(bcm63xx_usbd_dbg);
/*
* bcm63xx_iudma_dbg_show - Show IUDMA status and descriptors.
@@ -2238,33 +2239,7 @@ static int bcm63xx_iudma_dbg_show(struct seq_file *s,
void *p)
return 0;
}
-
-static int bcm63xx_usbd_dbg_open(struct inode *inode, struct file *file)
-{
- return single_open(file, bcm63xx_usbd_dbg_show, inode->i_private);
-}
-
-static int bcm63xx_iudma_dbg_open(struct inode *inode, struct file *file)
-{
- return single_open(file, bcm63xx_iudma_dbg_show, inode->i_private);
-}
-
-static const struct file_operations usbd_dbg_fops = {
- .owner = THIS_MODULE,
- .open = bcm63xx_usbd_dbg_open,
- .llseek = seq_lseek,
- .read = seq_read,
- .release= single_release,
-};
-
-static const struct file_operations iudma_dbg_fops = {
- .owner = THIS_MODULE,
- .open = bcm63xx_iudma_dbg_open,
- .llseek = seq_lseek,
- .read = seq_read,
- .release= single_release,
-};
-
+DEFINE_SHOW_ATTRIBUTE(bcm63xx_iudma_dbg);
/**
* bcm63xx_udc_init_debugfs - Create debugfs entries.
@@ -2282,11 +2257,11 @@ static void bcm63xx_udc_init_debugfs(struct bcm63xx_udc
*udc)
goto err_root;
usbd = debugfs_create_file("usbd", 0400, root, udc,
- _dbg_fops);
+ _usbd_dbg_fops);
if (!usbd)
goto err_usbd;
iudma = debugfs_create_file("iudma", 0400, root, udc,
- _dbg_fops);
+ _iudma_dbg_fops);
if (!iudma)
goto err_iudma;
--
2.15.1
--
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 v1 07/14] USB: gadget: pxa27x: Re-use DEFINE_SHOW_ATTRIBUTE() macro
...instead of open coding file operations followed by custom ->open()
callbacks per each attribute.
Cc: Daniel Mack <dan...@zonque.org>
Cc: Haojian Zhuang <haojian.zhu...@gmail.com>
Cc: Robert Jarzmik <robert.jarz...@free.fr>
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
drivers/usb/gadget/udc/pxa27x_udc.c | 42 +++--
1 file changed, 3 insertions(+), 39 deletions(-)
diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c
b/drivers/usb/gadget/udc/pxa27x_udc.c
index fadcf2653c3d..a58242e901df 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -131,6 +131,7 @@ static int state_dbg_show(struct seq_file *s, void *p)
return 0;
}
+DEFINE_SHOW_ATTRIBUTE(state_dbg);
static int queues_dbg_show(struct seq_file *s, void *p)
{
@@ -163,6 +164,7 @@ static int queues_dbg_show(struct seq_file *s, void *p)
return 0;
}
+DEFINE_SHOW_ATTRIBUTE(queues_dbg);
static int eps_dbg_show(struct seq_file *s, void *p)
{
@@ -199,45 +201,7 @@ static int eps_dbg_show(struct seq_file *s, void *p)
return 0;
}
-
-static int eps_dbg_open(struct inode *inode, struct file *file)
-{
- return single_open(file, eps_dbg_show, inode->i_private);
-}
-
-static int queues_dbg_open(struct inode *inode, struct file *file)
-{
- return single_open(file, queues_dbg_show, inode->i_private);
-}
-
-static int state_dbg_open(struct inode *inode, struct file *file)
-{
- return single_open(file, state_dbg_show, inode->i_private);
-}
-
-static const struct file_operations state_dbg_fops = {
- .owner = THIS_MODULE,
- .open = state_dbg_open,
- .llseek = seq_lseek,
- .read = seq_read,
- .release= single_release,
-};
-
-static const struct file_operations queues_dbg_fops = {
- .owner = THIS_MODULE,
- .open = queues_dbg_open,
- .llseek = seq_lseek,
- .read = seq_read,
- .release= single_release,
-};
-
-static const struct file_operations eps_dbg_fops = {
- .owner = THIS_MODULE,
- .open = eps_dbg_open,
- .llseek = seq_lseek,
- .read = seq_read,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(eps_dbg);
static void pxa_init_debugfs(struct pxa_udc *udc)
{
--
2.15.1
--
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 v1 06/14] USB: gadget: pxa25x: Re-use DEFINE_SHOW_ATTRIBUTE() macro
...instead of open coding file operations followed by custom ->open()
callbacks per each attribute.
Cc: Daniel Mack <dan...@zonque.org>
Cc: Haojian Zhuang <haojian.zhu...@gmail.com>
Cc: Robert Jarzmik <robert.jarz...@free.fr>
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
drivers/usb/gadget/udc/pxa25x_udc.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/gadget/udc/pxa25x_udc.c
b/drivers/usb/gadget/udc/pxa25x_udc.c
index 0e3f5faa000e..d4be53559f2e 100644
--- a/drivers/usb/gadget/udc/pxa25x_udc.c
+++ b/drivers/usb/gadget/udc/pxa25x_udc.c
@@ -1233,8 +1233,7 @@ static const struct usb_gadget_ops pxa25x_udc_ops = {
#ifdef CONFIG_USB_GADGET_DEBUG_FS
-static int
-udc_seq_show(struct seq_file *m, void *_d)
+static int udc_debug_show(struct seq_file *m, void *_d)
{
struct pxa25x_udc *dev = m->private;
unsigned long flags;
@@ -1335,25 +1334,12 @@ udc_seq_show(struct seq_file *m, void *_d)
local_irq_restore(flags);
return 0;
}
-
-static int
-udc_debugfs_open(struct inode *inode, struct file *file)
-{
- return single_open(file, udc_seq_show, inode->i_private);
-}
-
-static const struct file_operations debug_fops = {
- .open = udc_debugfs_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_ATTRIBUTE(udc_debug);
#define create_debug_files(dev) \
do { \
dev->debugfs_udc = debugfs_create_file(dev->gadget.name, \
- S_IRUGO, NULL, dev, _fops); \
+ S_IRUGO, NULL, dev, _debug_fops); \
} while (0)
#define remove_debug_files(dev) debugfs_remove(dev->debugfs_udc)
--
2.15.1
--
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 v1 12/14] USB: host: whci: Re-use DEFINE_SHOW_ATTRIBUTE() macro
...instead of open coding file operations followed by custom ->open()
callbacks per each attribute.
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
drivers/usb/host/whci/debug.c | 48 ++-
1 file changed, 6 insertions(+), 42 deletions(-)
diff --git a/drivers/usb/host/whci/debug.c b/drivers/usb/host/whci/debug.c
index f154e5791bfd..8ddfe3f1f693 100644
--- a/drivers/usb/host/whci/debug.c
+++ b/drivers/usb/host/whci/debug.c
@@ -72,7 +72,7 @@ static void qset_print(struct seq_file *s, struct whc_qset
*qset)
}
}
-static int di_print(struct seq_file *s, void *p)
+static int di_show(struct seq_file *s, void *p)
{
struct whc *whc = s->private;
int d;
@@ -91,8 +91,9 @@ static int di_print(struct seq_file *s, void *p)
}
return 0;
}
+DEFINE_SHOW_ATTRIBUTE(di);
-static int asl_print(struct seq_file *s, void *p)
+static int asl_show(struct seq_file *s, void *p)
{
struct whc *whc = s->private;
struct whc_qset *qset;
@@ -103,8 +104,9 @@ static int asl_print(struct seq_file *s, void *p)
return 0;
}
+DEFINE_SHOW_ATTRIBUTE(asl);
-static int pzl_print(struct seq_file *s, void *p)
+static int pzl_show(struct seq_file *s, void *p)
{
struct whc *whc = s->private;
struct whc_qset *qset;
@@ -118,45 +120,7 @@ static int pzl_print(struct seq_file *s, void *p)
}
return 0;
}
-
-static int di_open(struct inode *inode, struct file *file)
-{
- return single_open(file, di_print, inode->i_private);
-}
-
-static int asl_open(struct inode *inode, struct file *file)
-{
- return single_open(file, asl_print, inode->i_private);
-}
-
-static int pzl_open(struct inode *inode, struct file *file)
-{
- return single_open(file, pzl_print, inode->i_private);
-}
-
-static const struct file_operations di_fops = {
- .open= di_open,
- .read= seq_read,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
-
-static const struct file_operations asl_fops = {
- .open= asl_open,
- .read= seq_read,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
-
-static const struct file_operations pzl_fops = {
- .open= pzl_open,
- .read= seq_read,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_ATTRIBUTE(pzl);
void whc_dbg_init(struct whc *whc)
{
--
2.15.1
--
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 v1 08/14] USB: host: fhci: Re-use DEFINE_SHOW_ATTRIBUTE() macro
...instead of open coding file operations followed by custom ->open()
callbacks per each attribute.
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
drivers/usb/host/fhci-dbg.c | 26 ++
1 file changed, 2 insertions(+), 24 deletions(-)
diff --git a/drivers/usb/host/fhci-dbg.c b/drivers/usb/host/fhci-dbg.c
index fafa91189e45..ebf9bb219f75 100644
--- a/drivers/usb/host/fhci-dbg.c
+++ b/drivers/usb/host/fhci-dbg.c
@@ -55,6 +55,7 @@ static int fhci_dfs_regs_show(struct seq_file *s, void *v)
return 0;
}
+DEFINE_SHOW_ATTRIBUTE(fhci_dfs_regs);
static int fhci_dfs_irq_stat_show(struct seq_file *s, void *v)
{
@@ -75,30 +76,7 @@ static int fhci_dfs_irq_stat_show(struct seq_file *s, void
*v)
return 0;
}
-
-static int fhci_dfs_regs_open(struct inode *inode, struct file *file)
-{
- return single_open(file, fhci_dfs_regs_show, inode->i_private);
-}
-
-static int fhci_dfs_irq_stat_open(struct inode *inode, struct file *file)
-{
- return single_open(file, fhci_dfs_irq_stat_show, inode->i_private);
-}
-
-static const struct file_operations fhci_dfs_regs_fops = {
- .open = fhci_dfs_regs_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
-
-static const struct file_operations fhci_dfs_irq_stat_fops = {
- .open = fhci_dfs_irq_stat_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(fhci_dfs_irq_stat);
void fhci_dfs_create(struct fhci_hcd *fhci)
{
--
2.15.1
--
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 v1 01/14] USB: chipidea: Re-use DEFINE_SHOW_ATTRIBUTE() macro
...instead of open coding file operations followed by custom ->open()
callbacks per each attribute.
Cc: Peter Chen <peter.c...@nxp.com>
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
drivers/usb/chipidea/debug.c | 65
1 file changed, 5 insertions(+), 60 deletions(-)
diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c
index c9e1a165ed82..ce648cb3ed94 100644
--- a/drivers/usb/chipidea/debug.c
+++ b/drivers/usb/chipidea/debug.c
@@ -45,18 +45,7 @@ static int ci_device_show(struct seq_file *s, void *data)
return 0;
}
-
-static int ci_device_open(struct inode *inode, struct file *file)
-{
- return single_open(file, ci_device_show, inode->i_private);
-}
-
-static const struct file_operations ci_device_fops = {
- .open = ci_device_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(ci_device);
/**
* ci_port_test_show: reads port test mode
@@ -156,18 +145,7 @@ static int ci_qheads_show(struct seq_file *s, void *data)
return 0;
}
-
-static int ci_qheads_open(struct inode *inode, struct file *file)
-{
- return single_open(file, ci_qheads_show, inode->i_private);
-}
-
-static const struct file_operations ci_qheads_fops = {
- .open = ci_qheads_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(ci_qheads);
/**
* ci_requests_show: DMA contents of all requests currently queued (all endpts)
@@ -204,18 +182,7 @@ static int ci_requests_show(struct seq_file *s, void *data)
return 0;
}
-
-static int ci_requests_open(struct inode *inode, struct file *file)
-{
- return single_open(file, ci_requests_show, inode->i_private);
-}
-
-static const struct file_operations ci_requests_fops = {
- .open = ci_requests_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(ci_requests);
static int ci_otg_show(struct seq_file *s, void *unused)
{
@@ -278,18 +245,7 @@ static int ci_otg_show(struct seq_file *s, void *unused)
return 0;
}
-
-static int ci_otg_open(struct inode *inode, struct file *file)
-{
- return single_open(file, ci_otg_show, inode->i_private);
-}
-
-static const struct file_operations ci_otg_fops = {
- .open = ci_otg_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(ci_otg);
static int ci_role_show(struct seq_file *s, void *data)
{
@@ -376,18 +332,7 @@ static int ci_registers_show(struct seq_file *s, void
*unused)
return 0;
}
-
-static int ci_registers_open(struct inode *inode, struct file *file)
-{
- return single_open(file, ci_registers_show, inode->i_private);
-}
-
-static const struct file_operations ci_registers_fops = {
- .open = ci_registers_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(ci_registers);
/**
* dbg_create_files: initializes the attribute interface
--
2.15.1
--
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
