[PATCH] usb: phy: isp1301: Fix build warning when CONFIG_OF is disabled
Commit fd567653bdb9 ("usb: phy: isp1301: Add OF device ID table")
added an OF device ID table, but used the of_match_ptr() macro
that will lead to a build warning if CONFIG_OF symbol is disabled:
drivers/usb/phy//phy-isp1301.c:36:34: warning: ‘isp1301_of_match’ defined but
not used [-Wunused-const-variable=]
static const struct of_device_id isp1301_of_match[] = {
^~~~
Fixes: fd567653bdb9 ("usb: phy: isp1301: Add OF device ID table")
Reported-by: Arnd Bergmann
Signed-off-by: Javier Martinez Canillas
---
drivers/usb/phy/phy-isp1301.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/phy/phy-isp1301.c b/drivers/usb/phy/phy-isp1301.c
index b3b33cf7ddf6..f333024660b4 100644
--- a/drivers/usb/phy/phy-isp1301.c
+++ b/drivers/usb/phy/phy-isp1301.c
@@ -136,7 +136,7 @@ static int isp1301_remove(struct i2c_client *client)
static struct i2c_driver isp1301_driver = {
.driver = {
.name = DRV_NAME,
- .of_match_table = of_match_ptr(isp1301_of_match),
+ .of_match_table = isp1301_of_match,
},
.probe = isp1301_probe,
.remove = isp1301_remove,
--
2.9.3
--
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] usb: phy: isp1301: Add OF device ID table
Hello Felipe, On 03/28/2017 09:45 AM, Felipe Balbi wrote: > > Hi, > > Javier Martinez Canillas writes: >> The driver doesn't have a struct of_device_id table but supported devices >> are registered via Device Trees. This is working on the assumption that a >> I2C device registered via OF will always match a legacy I2C device ID and >> that the MODALIAS reported will always be of the form i2c:. >> >> But this could change in the future so the correct approach is to have an >> OF device ID table if the devices are registered via OF. >> >> Signed-off-by: Javier Martinez Canillas > > this is already upstream. Care to send an incremental diff in case we > need this fixed further? > Ah, sorry. I missed that was already picked. Sure, I'll post an incremental diff on top of v1. Thanks! Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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: phy: isp1301: Add OF device ID table
The driver doesn't have a struct of_device_id table but supported devices
are registered via Device Trees. This is working on the assumption that a
I2C device registered via OF will always match a legacy I2C device ID and
that the MODALIAS reported will always be of the form i2c:.
But this could change in the future so the correct approach is to have an
OF device ID table if the devices are registered via OF.
Signed-off-by: Javier Martinez Canillas
---
Changes in v2:
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
drivers/usb/phy/phy-isp1301.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/usb/phy/phy-isp1301.c b/drivers/usb/phy/phy-isp1301.c
index db68156568e6..f333024660b4 100644
--- a/drivers/usb/phy/phy-isp1301.c
+++ b/drivers/usb/phy/phy-isp1301.c
@@ -33,6 +33,12 @@ static const struct i2c_device_id isp1301_id[] = {
};
MODULE_DEVICE_TABLE(i2c, isp1301_id);
+static const struct of_device_id isp1301_of_match[] = {
+ {.compatible = "nxp,isp1301" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, isp1301_of_match);
+
static struct i2c_client *isp1301_i2c_client;
static int __isp1301_write(struct isp1301 *isp, u8 reg, u8 value, u8 clear)
@@ -130,6 +136,7 @@ static int isp1301_remove(struct i2c_client *client)
static struct i2c_driver isp1301_driver = {
.driver = {
.name = DRV_NAME,
+ .of_match_table = isp1301_of_match,
},
.probe = isp1301_probe,
.remove = isp1301_remove,
--
2.9.3
--
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] usb: phy: isp1301: Add OF device ID table
The driver doesn't have a struct of_device_id table but supported devices
are registered via Device Trees. This is working on the assumption that a
I2C device registered via OF will always match a legacy I2C device ID and
that the MODALIAS reported will always be of the form i2c:.
But this could change in the future so the correct approach is to have an
OF device ID table if the devices are registered via OF.
Signed-off-by: Javier Martinez Canillas
---
drivers/usb/phy/phy-isp1301.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/usb/phy/phy-isp1301.c b/drivers/usb/phy/phy-isp1301.c
index db68156568e6..b3b33cf7ddf6 100644
--- a/drivers/usb/phy/phy-isp1301.c
+++ b/drivers/usb/phy/phy-isp1301.c
@@ -33,6 +33,12 @@ static const struct i2c_device_id isp1301_id[] = {
};
MODULE_DEVICE_TABLE(i2c, isp1301_id);
+static const struct of_device_id isp1301_of_match[] = {
+ {.compatible = "nxp,isp1301" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, isp1301_of_match);
+
static struct i2c_client *isp1301_i2c_client;
static int __isp1301_write(struct isp1301 *isp, u8 reg, u8 value, u8 clear)
@@ -130,6 +136,7 @@ static int isp1301_remove(struct i2c_client *client)
static struct i2c_driver isp1301_driver = {
.driver = {
.name = DRV_NAME,
+ .of_match_table = of_match_ptr(isp1301_of_match),
},
.probe = isp1301_probe,
.remove = isp1301_remove,
--
2.9.3
--
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] usb: dwc3: exynos: Remove MODULE_ALIAS()
Exynos is DT-only, so there's no need for a platform MODALIAS.
Signed-off-by: Javier Martinez Canillas
---
drivers/usb/dwc3/dwc3-exynos.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index e27899bb5706..2e555a69c8ab 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -288,7 +288,6 @@ static struct platform_driver dwc3_exynos_driver = {
module_platform_driver(dwc3_exynos_driver);
-MODULE_ALIAS("platform:exynos-dwc3");
MODULE_AUTHOR("Anton Tikhomirov ");
MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("DesignWare USB3 EXYNOS Glue Layer");
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 0/6] inherit dma configuration from parent dev
[adding Arnd Bergmann who wrote the patches and I noticed was not cc'ed]
Hello,
On Tue, Jan 10, 2017 at 9:11 AM, Roger Quadros wrote:
> On 05/01/17 19:16, Greg KH wrote:
>> On Thu, Jan 05, 2017 at 05:21:09PM +0200, Roger Quadros wrote:
>>> Hi,
>>>
>>> On 17/11/16 13:43, Sriram Dash wrote:
For xhci-hcd platform device, all the DMA parameters are not
configured properly, notably dma ops for dwc3 devices.
The idea here is that you pass in the parent of_node along
with the child device pointer, so it would behave exactly
like the parent already does. The difference is that it also
handles all the other attributes besides the mask.
Arnd Bergmann (6):
usb: separate out sysdev pointer from usb_bus
usb: chipidea: use bus->sysdev for DMA configuration
usb: ehci: fsl: use bus->sysdev for DMA configuration
usb: xhci: use bus->sysdev for DMA configuration
usb: dwc3: use bus->sysdev for DMA configuration
usb: dwc3: Do not set dma coherent mask
>>>
>>> Only the dwc3 patches have been picked up for v4.10 and the rest are still
>>> missing.
>>> Any plans to pick them in the rc cycle?
>>>
>>> DMA configuration is still broken for XHCI on v4.10 and -next.
>>
>> It's always been broken, so it's not a big deal for 4.10 :)
>>
> OK. But I do see the following warning on xhci probe on TI platforms that use
> dwc3 IP. This is new in v4.10.
>
I see a similar warning on an Exynos5800 machine (Peach Pi). This is
new starting from v4.10-rc1.
> [ 11.664835] [ cut here ]
> [ 11.669693] WARNING: CPU: 1 PID: 534 at drivers/usb/host/xhci-plat.c:168
> xhci_plat_probe+0x180/0x450 [xhci_plat_hcd]
> [ 11.680765] Modules linked in: xhci_plat_hcd(+) xhci_hcd usbcore evdev
> dwc3 snd_soc_simple_card snd_soc_tlv320aic3x snd_soc_simple_card_utils
> udc_core m25p80 snd_soc_davinci_mcasp usb_common spi_nor snd_soc_e
> [ 11.726078] CPU: 1 PID: 534 Comm: modprobe Tainted: GW
> 4.10.0-rc3 #1075
> [ 11.734293] Hardware name: Generic DRA74X (Flattened Device Tree)
> [ 11.740695] [] (unwind_backtrace) from []
> (show_stack+0x10/0x14)
> [ 11.748829] [] (show_stack) from []
> (dump_stack+0xac/0xe0)
> [ 11.756413] [] (dump_stack) from [] (__warn+0xd8/0x104)
> [ 11.763722] [] (__warn) from []
> (warn_slowpath_null+0x20/0x28)
> [ 11.771675] [] (warn_slowpath_null) from []
> (xhci_plat_probe+0x180/0x450 [xhci_plat_hcd])
> [ 11.782110] [] (xhci_plat_probe [xhci_plat_hcd]) from
> [] (platform_drv_probe+0x4c/0xb0)
> [ 11.792338] [] (platform_drv_probe) from []
> (driver_probe_device+0x200/0x2d4)
> [ 11.801655] [] (driver_probe_device) from []
> (__driver_attach+0xc0/0xc4)
> [ 11.810518] [] (__driver_attach) from []
> (bus_for_each_dev+0x6c/0xa0)
> [ 11.819095] [] (bus_for_each_dev) from []
> (bus_add_driver+0x18c/0x214)
> [ 11.827767] [] (bus_add_driver) from []
> (driver_register+0x78/0xf8)
> [ 11.836167] [] (driver_register) from []
> (do_one_initcall+0x3c/0x174)
> [ 11.844751] [] (do_one_initcall) from []
> (do_init_module+0x5c/0x1d0)
> [ 11.853247] [] (do_init_module) from []
> (load_module+0x1d48/0x23a8)
> [ 11.861649] [] (load_module) from []
> (SyS_init_module+0x150/0x174)
> [ 11.869961] [] (SyS_init_module) from []
> (ret_fast_syscall+0x0/0x1c)
> [ 11.878494] ---[ end trace a0ca4c6c0c9a9ee9 ]---
> [ 11.883400] xhci-hcd xhci-hcd.0.auto: xHCI Host Controller
>
I tracked down to commit d64ff406e51e ("usb: dwc3: use bus->sysdev for
DMA configuration") which is part of this series. If I revert that
commit, the warning goes away.
Best regards,
Javier
--
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 dwc3_exynos_probe() change goto labels to meaningful names
Hello Shuah,
On 01/11/2017 01:45 PM, Shuah Khan wrote:
> Change goto labels to meaningful names from a series of errNs.
>
> Signed-off-by: Shuah Khan
> ---
> drivers/usb/dwc3/dwc3-exynos.c | 22 +++---
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index f7421c2..ea50f74 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -147,53 +147,53 @@ static int dwc3_exynos_probe(struct platform_device
> *pdev)
> exynos->vdd33 = devm_regulator_get(dev, "vdd33");
> if (IS_ERR(exynos->vdd33)) {
> ret = PTR_ERR(exynos->vdd33);
> - goto err2;
> + goto vdd33_err;
> }
I think it's better to use as labels what will be done in the error path
rather than what failed. IOW, "disable_axius_clk" or "disable_clks" are
more clear than vdd33_err for me, since can give you an idea of what will
happen in the error path.
But I don't have a strong opinion and your naming is an improvement over
the current one, so:
Reviewed-by: Javier Martinez Canillas
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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-exynos fix axius clock error path to do cleanup
Hello Shuah, On 01/10/2017 09:30 PM, Shuah Khan wrote: [snip] >>> clk_disable_unprepare(exynos->axius_clk); >>> +axius_clk_err: >> >> This label isn't consistent with the others, I know the errN aren't great >> so what about changing those to meaningful names in a preparatory patch? >> >> Reviewed-by: Javier Martinez Canillas >> >> Best regards, >> > > Javier, > > Right they aren't consistent. Changing them all to a better naming scheme > will have be done in another cleanup patch in my opinion. I don't want to > include cleanup in this fix. > I didn't mean to be done in the same patch, that's why I said in another preparatory patch. > thanks, > -- Shuah > Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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-exynos fix axius clock error path to do cleanup
Hello Shuah,
Patch looks good to me, I've just one comment.
On 01/10/2017 08:05 PM, Shuah Khan wrote:
> Axius clock error path returns without disabling clock and suspend clock.
> Fix it to disable them before returning error.
>
> Signed-off-by: Shuah Khan
> ---
> drivers/usb/dwc3/dwc3-exynos.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index 3e8407a..f7421c2 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -136,7 +136,8 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> exynos->axius_clk = devm_clk_get(dev, "usbdrd30_axius_clk");
> if (IS_ERR(exynos->axius_clk)) {
> dev_err(dev, "no AXI UpScaler clk specified\n");
> - return -ENODEV;
> + ret = -ENODEV;
> + goto axius_clk_err;
> }
> clk_prepare_enable(exynos->axius_clk);
> } else {
> @@ -194,6 +195,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> regulator_disable(exynos->vdd33);
> err2:
> clk_disable_unprepare(exynos->axius_clk);
> +axius_clk_err:
This label isn't consistent with the others, I know the errN aren't great
so what about changing those to meaningful names in a preparatory patch?
Reviewed-by: Javier Martinez Canillas
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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-exynos remove suspend clock unspecified debug message
Hello Shuah,
On 01/10/2017 06:20 PM, Shuah Khan wrote:
> dwc3-exynos prints debug message when suspend clock is not specified.
> The suspend clock is optional and driver can work without it.
>
> This debug message doesn't add any value and leads to confusion and
> concern. Remove it.
>
> Signed-off-by: Shuah Khan
> ---
> This patch is a result of the disussion on the following patch.
> https://lkml.org/lkml/2017/1/9/891
>
> drivers/usb/dwc3/dwc3-exynos.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index e27899b..3e8407a 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -128,10 +128,8 @@ static int dwc3_exynos_probe(struct platform_device
> *pdev)
> clk_prepare_enable(exynos->clk);
>
> exynos->susp_clk = devm_clk_get(dev, "usbdrd30_susp_clk");
> - if (IS_ERR(exynos->susp_clk)) {
> - dev_info(dev, "no suspend clk specified\n");
Sorry for being late in the discussion, but what about making it a
dev_dbg() instead? That way the message may not confuse others but
still keep information that could be useful for debugging purposes.
I know the current message isn't great but keeping it as debug allows
to improve it later.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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 v1.1] ARM: multi_v7_defconfig: Enable power sequence for Odroid U3
Hello Krzysztof, On 01/09/2017 02:38 PM, Krzysztof Kozlowski wrote: > On Mon, Jan 9, 2017 at 6:24 PM, Javier Martinez Canillas > wrote: >> Hello Krzysztof, >> >> I think it would had been clearer if the subject prefix was "[PATCH v1.1 >> 4/4]" :) > > Ah, yes. > >> >> On 01/07/2017 06:16 AM, Krzysztof Kozlowski wrote: >>> Odroid U3 needs a power sequence for lan9730, if it was enabled by >>> bootloader. Also enable the USB3503 HSCI to USB2.0 driver (device >>> is present on Odroid U3). >>> >>> Signed-off-by: Krzysztof Kozlowski >>> >>> --- >>> >> >> Do you think that makes sense to also enable GPIO_SYS for debugging >> purposes as you do in patch 3/4? > > I like the GPIO_SYSFS interface because it is easy to use. However > now, after your question, I found this: > http://lists.infradead.org/pipermail/linux-arm-kernel/2016-November/470154.html > ARM/ARM64: defconfig: drop GPIO_SYSFS on multiplatforms > > ... so instead I will drop it from exynos_defconfig. > Right, I forgot that using the GPIO sysfs interface is discouraged now and the new chardev/ioctl based should be used instead. So it makes sense to drop it. > Best regards, > Krzysztof Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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 v1.1] ARM: multi_v7_defconfig: Enable power sequence for Odroid U3
Hello Krzysztof, I think it would had been clearer if the subject prefix was "[PATCH v1.1 4/4]" :) On 01/07/2017 06:16 AM, Krzysztof Kozlowski wrote: > Odroid U3 needs a power sequence for lan9730, if it was enabled by > bootloader. Also enable the USB3503 HSCI to USB2.0 driver (device > is present on Odroid U3). > > Signed-off-by: Krzysztof Kozlowski > > --- > Do you think that makes sense to also enable GPIO_SYS for debugging purposes as you do in patch 3/4? In any case the patch looks good to me: Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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/4] ARM: dts: exynos: Fix LAN9730 on Odroid U3 after tftpboot
Hello Krzysztof, On 01/07/2017 05:52 AM, Krzysztof Kozlowski wrote: > The ethernet adapter LAN9730, after enabling in bootloader (e.g. for > tftpboot) requires reset during boot. Otherwise it won't come up. > > The schematics of Odroid U3 are detailed enough but after grabbing > knowledge also from other sources (like U-Boot) the overall design looks > like: > 1. LAN9730 is connected to HSIC0 and USB3503 to HSIC1 of EHCI controller. > 2. USB3503 comes with its own reset pin: gpx3-5. > 3. Reset pin of LAN9730 is pulled up to 3.3 V so it cannot be used. > 4. The supply of 3.3 V for LAN9730 is delivered from buck8. > 5. Buck8 state is a logical OR of registry value (through I2C command) >and ENB8 pin. The ENB8, not described in schematics, is in fact >gpa1-1. > 6. Missing or wrongly timed reset of LAN9730 might result in missing of >two devices: LAN9730 and USB3503. Without reset, LAN9730 will not >come up, if it was enabled by bootloader. > > To fix the issue use the generic power sequence driver and toggle the > ENB8 (buck8) pin. Reset duration of 500 us was chosen by experiments > (shortest working time was 400 us). This is an easiest way to fix the > long standing LAN9730 reset issue. > > Signed-off-by: Krzysztof Kozlowski > --- Thanks for keep pushing a fix for this long standing issue. Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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/4] ARM: exynos_defconfig: Enable power sequence for Odroid U3
Hello Krzysztof, On 01/07/2017 05:52 AM, Krzysztof Kozlowski wrote: > Odroid U3 needs a power sequence for lan9730, if it was enabled by > bootloader. Enable also GPIO_SYSFS which is useful for playing with > GPIO during debug process. > > Signed-off-by: Krzysztof Kozlowski > --- Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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/4] ARM: dts: exynos: Fix indentation of EHCI and OHCI ports
Hello Krzysztof, On 01/07/2017 05:52 AM, Krzysztof Kozlowski wrote: > Replace spaces with tabs in EHCI and OHCI ports indentation. > > Signed-off-by: Krzysztof Kozlowski > --- Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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] usb: host: ehci-exynos: Decrese node refcount on exynos_ehci_get_phy() error paths
Hello Krzysztof,
On 01/07/2017 05:41 AM, Krzysztof Kozlowski wrote:
> Returning from for_each_available_child_of_node() loop requires cleaning
> up node refcount. Error paths lacked it so for example in case of
> deferred probe, the refcount of phy node was left increased.
>
> Fixes: 6d40500ac9b6 ("usb: ehci/ohci-exynos: Fix of_node_put() for child when
> getting PHYs")
> Signed-off-by: Krzysztof Kozlowski
> ---
Reviewed-by: Javier Martinez Canillas
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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/2] usb: host: ohci-exynos: Decrese node refcount on exynos_ehci_get_phy() error paths
Hello Krzysztof,
On 01/07/2017 05:41 AM, Krzysztof Kozlowski wrote:
> Returning from for_each_available_child_of_node() loop requires cleaning
> up node refcount. Error paths lacked it so for example in case of
> deferred probe, the refcount of phy node was left increased.
>
> Fixes: 6d40500ac9b6 ("usb: ehci/ohci-exynos: Fix of_node_put() for child when
> getting PHYs")
> Signed-off-by: Krzysztof Kozlowski
> ---
Reviewed-by: Javier Martinez Canillas
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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 02/23] of: device: Export of_device_{get_modalias, uvent_modalias} to modules
Hello ChenYu On Fri, Nov 11, 2016 at 12:01 AM, Chen-Yu Tsai wrote: > On Fri, Nov 11, 2016 at 5:42 AM, Rob Herring wrote: [snip] >> >> Do you have patches using this already. If not, it is starting to get >> a bit late for v4.10. >> >> I can apply this, but then you'll just be pulling in other DT patches. > > Not sure what you mean by "using" this... > > I have patches which use this to add DT-based modalias entries for > module auto-loading to i2c and sunxi-rsb that I haven't sent. > Unfortunately the I2C core can't be changed without breaking a lot of I2C drivers that are relying on the current behavior. I've already posted a RFC patch [0] for I2C that does this about a year ago and enumerated the issues that have to be addressed before the change can be made (and fixed some of the issues mentioned) on this series [1]. Another issue is that an I2C device ID table is a requirement anyways since I2C drivers expect an i2c_device_id as an argument of their probe function. Kieran already have patches [2] to change that which should land soon. I plan to fix the remaining I2C drivers once his patches are merged and re-post the RFC patch as a proper one. > As far as DT usage goes, we already need this for the axp20x mfd driver. > There are 2 variants, i2c and sunxi-rsb. For the I2C variant a fix was > sent to fix module auto-loading by using the I2C client ID table: > > mfd: axp20x-i2c: Add i2c-ids to fix module auto-loading > > https://git.kernel.org/cgit/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=b7142a19321484bd7681aa547c1d50148c8e2825 > Yes, this is the workaround used by most DT-only I2C drivers. The only reason that these drivers have an I2C device ID is due the restrictions imposed by the I2C core. Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] usb: core: use IS_ENABLED() instead of checking for built-in or module
The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.
Using the macro makes the code more readable by helping abstract away some
of the Kconfig built-in and module enable details.
Signed-off-by: Javier Martinez Canillas
---
drivers/usb/core/hcd.c | 2 +-
drivers/usb/core/otg_whitelist.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index d2e3f655c26f..0e2c733d20c3 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -3033,7 +3033,7 @@ EXPORT_SYMBOL_GPL(usb_hcd_platform_shutdown);
/*-*/
-#if defined(CONFIG_USB_MON) || defined(CONFIG_USB_MON_MODULE)
+#if IS_ENABLED(CONFIG_USB_MON)
const struct usb_mon_operations *mon_ops;
diff --git a/drivers/usb/core/otg_whitelist.h b/drivers/usb/core/otg_whitelist.h
index a95b0c989c21..085049d37d7a 100644
--- a/drivers/usb/core/otg_whitelist.h
+++ b/drivers/usb/core/otg_whitelist.h
@@ -38,7 +38,7 @@ static struct usb_device_id whitelist_table[] = {
{ USB_DEVICE(0x0525, 0xa4a2), },
#endif
-#ifdefined(CONFIG_USB_TEST) || defined(CONFIG_USB_TEST_MODULE)
+#ifIS_ENABLED(CONFIG_USB_TEST)
/* gadget zero, for testing */
{ USB_DEVICE(0x0525, 0xa4a0), },
#endif
--
2.5.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] usb: gadget: use IS_ENABLED() instead of checking for built-in or module
The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either built-in or as a module, use that macro instead of open coding the same. Using the macro makes the code more readable by helping abstract away some of the Kconfig built-in and module enable details. Signed-off-by: Javier Martinez Canillas --- drivers/usb/gadget/udc/omap_udc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index 9b7d39484ed3..a8709f9e5648 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2875,7 +2875,7 @@ bad_on_1710: xceiv = NULL; /* "udc" is now valid */ pullup_disable(udc); -#ifdefined(CONFIG_USB_OHCI_HCD) || defined(CONFIG_USB_OHCI_HCD_MODULE) +#ifIS_ENABLED(CONFIG_USB_OHCI_HCD) udc->gadget.is_otg = (config->otg != 0); #endif -- 2.5.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] USB: serial: use IS_ENABLED() instead of checking for built-in or module
The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either built-in or as a module, use that macro instead of open coding the same. Using the macro makes the code more readable by helping abstract away some of the Kconfig built-in and module enable details. Signed-off-by: Javier Martinez Canillas --- drivers/usb/serial/keyspan_pda.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index 4f7e072e4e00..e49ad0c63ad8 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -30,12 +30,12 @@ #include /* make a simple define to handle if we are compiling keyspan_pda or xircom support */ -#if defined(CONFIG_USB_SERIAL_KEYSPAN_PDA) || defined(CONFIG_USB_SERIAL_KEYSPAN_PDA_MODULE) +#if IS_ENABLED(CONFIG_USB_SERIAL_KEYSPAN_PDA) #define KEYSPAN #else #undef KEYSPAN #endif -#if defined(CONFIG_USB_SERIAL_XIRCOM) || defined(CONFIG_USB_SERIAL_XIRCOM_MODULE) +#if IS_ENABLED(CONFIG_USB_SERIAL_XIRCOM) #define XIRCOM #else #undef XIRCOM -- 2.5.5 -- 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 v2 13/13] ARM: dts: exynos: Fix LAN and HUB after bootloader initialization on Odroid U3
Hello Krzysztof,
On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> On Odroid U3 (Exynos4412-based) board if USB was initialized by
> bootloader (in U-Boot "usb start" before tftpboot), the HUB (usb3503)
> and LAN (smsc95xx) after after successful probing were not visible in the
> system ("lsusb").
>
> In such case the devices had to be fully reset before configuring.
> Reset by GPIO (called RESET_N pin) and by RESET field in STCD register
> in usb3503 HUB are not sufficient. Instead full reset has to be done by
> disabling and enabling regulator.
>
> Signed-off-by: Krzysztof Kozlowski
> ---
[snip]
>
> + lan_pwrseq: pwrseq2 {
> + compatible = "mmc-pwrseq-simple";
It feels strange to have a "mmc-pwrseq-simple" compatible for a USB power
sequence provider. As I mentioned in the other patch, I think there should
either be a DT binding for the USB pwrseq-simple with a "usb-pwrseq-simple"
compatible that binds to the same pwrseq-simple driver or maybe having a
generic DT binding for any device with a new "pwrseq-simple" compatible.
Patch looks good to me though, so after having a DT binding and changing
the compatible string:
Reviewed-by: Javier Martinez Canillas
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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 v2 12/13] ARM: dts: exynos: Switch the buck8 to GPIO mode on Odroid U3
Hello Krzysztof, On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote: > Switch the control of buck8 to GPIO mode. It is faster than I2C/register > mode and it is the easiest way to disable it (regulator state is a > logical OR state of GPIO and register value). > > Signed-off-by: Krzysztof Kozlowski > --- Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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 v2 11/13] usb: port: Parse pwrseq phandle from Device Tree
Hello Krzysztof,
On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Parse usb-pwrseq property from Device Tree to get the phandle to pwrseq
> device. The pwrseq device will be used by USB hub to cycle the power
> before activating ports.
>
> Signed-off-by: Krzysztof Kozlowski
> ---
[snip]
>
> @@ -532,6 +534,14 @@ int usb_hub_create_port_device(struct usb_hub *hub, int
> port1)
> return retval;
> }
>
> + port_dev->dev.of_node = usb_of_get_child_node(hub->hdev->dev.of_node,
> port1);
> + port_dev->pwrseq = pwrseq_alloc(&port_dev->dev);
> + if (IS_ERR(port_dev->pwrseq)) {
> + device_unregister(&port_dev->dev);
> + /* TODO: what about EPROBE_DEFER? */
I think it's OK since the call chain is:
hub_probe()
hub_configure()
usb_hub_create_port_device()
so the hub_probe() will be deferred if the usb-pwrseq was not registered yet.
Unless I misunderstood your question :)
Anyway, patch looks good to me:
Reviewed-by: Javier Martinez Canillas
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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 v2 10/13] usb: hub: Power sequence the ports on activation
Hello Krzysztof, On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote: > The autodetection of attached USB device might not work on certain > boards where the power is delivered externally. These devices also might > require a hard reset. Use pwrseq for that in USB hub. > > Signed-off-by: Krzysztof Kozlowski > --- Patch looks good to me. So after fixing the bisectability issue pointed out by Alan Stern: Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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 v2 09/13] power: pwrseq: Add support for USB hubs with external power
Hello Krzysztof,
On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Some USB devices on embedded boards have external power supply which has
> to be reset in certain conditions. Add pwrseq interface for this.
>
> Signed-off-by: Krzysztof Kozlowski
> ---
> drivers/power/pwrseq/pwrseq.c | 44
> +++
> include/linux/pwrseq.h| 8
> 2 files changed, 52 insertions(+)
>
> diff --git a/drivers/power/pwrseq/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
> index 495a19d3c30b..306265f55a10 100644
> --- a/drivers/power/pwrseq/pwrseq.c
> +++ b/drivers/power/pwrseq/pwrseq.c
> @@ -52,6 +52,43 @@ int mmc_pwrseq_alloc(struct mmc_host *host)
> }
> EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
>
> +struct pwrseq *pwrseq_alloc(struct device *dev)
> +{
This function is USB specific so better to call it usb_pwrseq_alloc() instead.
Although, this function has a lot of duplicated code from mmc_pwrseq_alloc()
so I think is better to keep the name generic and factorize the common code.
I expect other devices are also needing some kind of power seq in the future
so having a single alloc function instead of each for device type makes sense.
> + struct device_node *np;
> + struct pwrseq *p, *ret = NULL;
> +
> + np = of_parse_phandle(dev->of_node, "usb-pwrseq", 0);
I know this is just an RFC but you should really add DT bindings for this.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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 v2 08/13] usb: hub: Handle deferred probe
Hello Krzysztof, On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote: > Add support for deferred probing to the usb hub. Currently EPROBE_DEFER > does not exist in usb hub path but future patches will add it on the > port level. > > Signed-off-by: Krzysztof Kozlowski > --- Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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 v2 07/13] power: pwrseq: simple: Add support for toggling regulator
Hello Krzysztof,
On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Some devices need real hard-reset by cutting the power. During power
> sequence turn off and on the regulator, if it is provided.
>
> Signed-off-by: Krzysztof Kozlowski
> ---
[snip]
>
> #define to_pwrseq_simple(p) container_of(p, struct mmc_pwrseq_simple, pwrseq)
> @@ -62,6 +65,13 @@ static void mmc_pwrseq_simple_post_power_on(struct pwrseq
> *_pwrseq)
> {
> struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
>
> + if (pwrseq->ext_reg) {
> + int err;
> +
> + err = regulator_enable(pwrseq->ext_reg);
> + WARN_ON_ONCE(err);
> + }
> +
Shouldn't this be in mmc_pwrseq_simple_pre_power_on() instead?
For example, a chip may need to be powered on before attempting to
toggle its reset or power pins using some GPIO lines.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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 v2 06/13] power: pwrseq: Generalize mmc_pwrseq operations by removing mmc prefix
Hello Krzysztof, On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote: > The power sequence hooks (mmc_pwrseq_pre_power_on(), > mmc_pwrseq_post_power_on() and mmc_pwrseq_power_off()) can be made more > generic to allow re-use in other subsystems. They do not need to take > pointer to struct mmc_host but instead the struct pwrseq should be > sufficient. > > Remove the "mmc" prefix and use the pointer to struct pwrseq as > argument. > > Signed-off-by: Krzysztof Kozlowski > --- Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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 v2 05/13] power: pwrseq: Remove mmc prefix from mmc_pwrseq
Hello Krzysztof, On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote: > The "mmc" prefix is no longer needed after moving the pwrseq core code > from mmc/ to power/. > > Signed-off-by: Krzysztof Kozlowski > --- [snip] > diff --git a/drivers/power/pwrseq/pwrseq_emmc.c > b/drivers/power/pwrseq/pwrseq_emmc.c > index a0583ed46d7f..a68ac9a68e04 100644 > --- a/drivers/power/pwrseq/pwrseq_emmc.c > +++ b/drivers/power/pwrseq/pwrseq_emmc.c > @@ -22,7 +22,7 @@ > #include > I don't think this header inclusion is needed. At least I didn't see anything defined there that's used in this driver. This also applies to pwrseq_simple. I think you could remove those in another preparatory patches before the move. Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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 v2 04/13] power: pwrseq: Enable COMPILE_TEST for drivers
Hello Krzysztof, On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote: > Allow build testing for power sequence drivers. > > Signed-off-by: Krzysztof Kozlowski > --- Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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 v2 03/13] MAINTAINERS: Retain Ulf Hansson as the same maintainer of pwrseq
Krzysztof, On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote: > Before moving pwrseq drivers from drivers/mmc/core/ to drivers/power/, > they were maintained by Ulf Hansson. > > Signed-off-by: Krzysztof Kozlowski > --- Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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 v2 02/13] power/mmc: Move pwrseq drivers to power/pwrseq
Hello Krzysztof, On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote: > The MMC power sequence drivers are useful also outside of MMC world: for > USB devices needed a hard-reset before probing. Before extending and > re-using pwrseq drivers, move them to a new place. > > The commit does not introduce significant changes in the pwrseq drivers > code so still all the functions are prefixed with "mmc_pwrseq". However > the MMC-specific pwrseq functions has to be now exported and everything > is hidden not by CONFIG_OF but by new CONFIG_POWER_SEQ option. > > Signed-off-by: Krzysztof Kozlowski > --- [snip] > --- a/drivers/mmc/core/Kconfig > +++ b/drivers/power/pwrseq/Kconfig > @@ -1,7 +1,12 @@ > -# > -# MMC core configuration > -# > -config PWRSEQ_EMMC > +menuconfig POWER_SEQ > + default y if OF > + bool "Hardware reset support for specific devices" > + help > + Provides drivers which reset the specific device before... > + I think this text could be improved a little bit, maybe something like: "Provides drivers that implements specific power sequences for chips, using the generic power sequence management interface". The rest looks good to me. Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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 v2 01/13] usb: misc: usb3503: Clean up on driver unbind
Hello Krzysztof
Patch looks good to me, I just have a trivial comment below.
On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> The driver should clean up after itself by unpreparing the clock when it
> is unbound.
>
> Signed-off-by: Krzysztof Kozlowski
> ---
> drivers/usb/misc/usb3503.c | 28
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c
> index b45cb77c0744..a464636675a6 100644
> --- a/drivers/usb/misc/usb3503.c
> +++ b/drivers/usb/misc/usb3503.c
> @@ -330,6 +330,19 @@ static int usb3503_i2c_probe(struct i2c_client *i2c,
> return usb3503_probe(hub);
> }
>
> +static int usb3503_i2c_remove(struct i2c_client *i2c)
> +{
> + struct usb3503 *hub;
> +
> + hub = i2c_get_clientdata(i2c);
> + if (hub) {
> + if (hub->clk)
> + clk_disable_unprepare(hub->clk);
> + }
I think the following is simpler and easier to read:
if (hub && hub->clk)
clk_disable_unprepare(hub->clk);
> +
> + return 0;
> +}
> +
> static int usb3503_platform_probe(struct platform_device *pdev)
> {
> struct usb3503 *hub;
> @@ -342,6 +355,19 @@ static int usb3503_platform_probe(struct platform_device
> *pdev)
> return usb3503_probe(hub);
> }
>
> +static int usb3503_platform_remove(struct platform_device *pdev)
> +{
> + struct usb3503 *hub;
> +
> + hub = platform_get_drvdata(pdev);
> + if (hub) {
> + if (hub->clk)
> + clk_disable_unprepare(hub->clk);
> + }
> +
Ditto.
> + return 0;
> +}
> +
> #ifdef CONFIG_PM_SLEEP
> static int usb3503_i2c_suspend(struct device *dev)
> {
> @@ -395,6 +421,7 @@ static struct i2c_driver usb3503_i2c_driver = {
> .of_match_table = of_match_ptr(usb3503_of_match),
> },
> .probe = usb3503_i2c_probe,
> + .remove = usb3503_i2c_remove,
> .id_table = usb3503_id,
> };
>
> @@ -404,6 +431,7 @@ static struct platform_driver usb3503_platform_driver = {
> .of_match_table = of_match_ptr(usb3503_of_match),
> },
> .probe = usb3503_platform_probe,
> + .remove = usb3503_platform_remove,
> };
>
> static int __init usb3503_init(void)
>
Reviewed-by: Javier Martinez Canillas
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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] USB: serial: Use IS_ENABLED() instead of checking for FOO || FOO_MODULE
The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either built-in or as a module, use that macro instead of open coding the same. Signed-off-by: Javier Martinez Canillas --- drivers/usb/serial/usb-serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index 46f1f13b41f1..7ecf4ff86b9a 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -815,7 +815,7 @@ static int usb_serial_probe(struct usb_interface *interface, } } -#if defined(CONFIG_USB_SERIAL_PL2303) || defined(CONFIG_USB_SERIAL_PL2303_MODULE) +#if IS_ENABLED(CONFIG_USB_SERIAL_PL2303) /* BEGIN HORRIBLE HACK FOR PL2303 */ /* this is needed due to the looney way its endpoints are set up */ if (((le16_to_cpu(dev->descriptor.idVendor) == PL2303_VENDOR_ID) && -- 2.5.5 -- 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] musb: sunxi-glue: Fix sunxi-musb driver not auto-loading
On Mon, Mar 21, 2016 at 7:13 PM, Hans de Goede wrote: > Add a missing MODULE_DEVICE_TABLE() line which was causing the > sunxi-musb glue to not auto-load when build as a module. > > Signed-off-by: Hans de Goede > --- Reviewed-by: Javier Martinez Canillas Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] usb: musb: sunxi: support module autoloading
Hello Emilio, On Sun, Feb 21, 2016 at 10:26 PM, Emilio López wrote: > From: Emilio López > > MODULE_DEVICE_TABLE() is missing, so the module isn't auto-loading on > sunxi systems using the OTG controller. This commit adds the missing > line so it loads automatically when building it as a module and running > on a system with an USB OTG port. > > Signed-off-by: Emilio López > --- Reviewed-by: Javier Martinez Canillas Best regards, Javier -- 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] dmaengine: sun4i: support module autoloading
Hello Emilio, On Sun, Feb 21, 2016 at 10:26 PM, Emilio López wrote: > From: Emilio López > > MODULE_DEVICE_TABLE() is missing, so the module isn't auto-loading on > supported systems. This commit adds the missing line so it loads > automatically when building it as a module and running on a system > with the early sunxi DMA engine. > > Signed-off-by: Emilio López > --- Reviewed-by: Javier Martinez Canillas Best regards, Javier -- 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] [media] rc: sunxi-cir: support module autoloading
Hello Emilio, On Sun, Feb 21, 2016 at 10:26 PM, Emilio López wrote: > From: Emilio López > > MODULE_DEVICE_TABLE() is missing, so the module isn't auto-loading on > systems supporting infrared. This commit adds the missing line so it > works out of the box when built as a module and running on a sunxi > system with an infrared receiver. > > Signed-off-by: Emilio López Looks good to me. Reviewed-by: Javier Martinez Canillas Best regards, Javier -- 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: [RFT 0/3] usb: usb3503: Fix probing on Arndale board (missing phy)
Hello, On 10/08/2015 08:23 AM, Marek Szyprowski wrote: > Hello, > > On 2015-10-08 08:02, Krzysztof Kozlowski wrote: >> On 07.10.2015 23:26, Marek Szyprowski wrote: >>> Hello, >>> >>> On 2015-10-07 02:30, Krzysztof Kozlowski wrote: >>>> Introduction >>>> >>>> This patchset tries to fix probing of usb3503 on Arndale board >>>> if the Samsung PHY driver is probed later (or built as a module). >>>> >>>> *The patchset was not tested on Arndale board.* >>>> I don't have that board. Please test it and say if the usb3503 >>>> deferred probe >>>> works fine and the issue is solved. >>>> >>>> The patchset was tested on Odroid U3 board (which is different!) >>>> in a simulated environment. It is not sufficient testing. >>>> >>>> >>>> Difference >>>> == >>>> The usb3503 device driver can be used as a I2C device (on Odroid U3) >>>> or as a platform device connected through phy (on Arndale). In the second >>>> case the necessary phy reference has to be obtained and enabled. >>>> >>>> For some details please look also at thread [0][1]. >>>> >>>> [0] >>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/348524.html >>>> >>>> [1] >>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/348875.html >>>> >>>> >>> I'm not sure that this is the correct approach. usb3503 chip is simply >>> connected >>> to Exynos USB2 phy, so it visible on the USB bus. The real driver that >>> controls USB2 >>> PHY is Exynos EHCI driver and USB3503 should not mess around it. >> The ehci node (usb@1211) has one port configured and it takes one >> PHY reference (phy of id 1 - USB host). I can't see driver taking >> reference to HSIC0 or HSIC1 phys... Since I cannot diagnose the error I >> don't know what is really expected here. > > It looks that EHCI in Exynos 5250 and 5420 still use old phy bindings. For > the reference, see Exynos4 dts and exynos4412-odroidu3.dts to check how to > enable > more than one USB port (Odroid U3 has both HSIC ports enabled). > >> >>> In my opinion all that is needed in case of Arndale board is forcing >>> reset of >>> usb3503 chip after successful EHCI and USB2 PHY initialization (for some >>> reason >>> initialization of usb3503 chip must be done after usb host initialization). >>> However I have no idea which driver should trigger this reset. Right now >>> I didn't >>> find any good solution for additional control for devices which are on >>> autoprobed >>> bus like usb. >> The reset is done at the end of usb3503's probe. The question "why >> usb3503 has to be initialized after EHCI and USB PHY" is still valid... > > I remember that I saw some code to reset HSIC device after phy power on in > case > of HSIC-connected modem chip, so maybe this is somehow common for HSIC chips > (which are some special case of 'embedded usb'). > I also don't have an Arndale board and haven't followed the thread to closely but I just wanted to mention that the ChromiumOS 3.8 tree has a workaround to reset the HSIC phys: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/81685c447954a29d1098268776582457258dd98f%5E%21/ and later a "supports-hsicphy-reset" DT property was added to force the reset per board instead of unconditionally: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/a4d1c1a223ffa1ed38a4257d0378ca70c6667be0%5E%21/ Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] mmc: vub300: Remove unneded semicolons
They aren't needed and are just creating null statements so remove it. Signed-off-by: Javier Martinez Canillas --- drivers/mmc/host/vub300.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c index fbabbb82b354..1e819f98b94f 100644 --- a/drivers/mmc/host/vub300.c +++ b/drivers/mmc/host/vub300.c @@ -563,7 +563,7 @@ static void add_offloaded_reg(struct vub300_mmc_host *vub300, i += 1; continue; } - }; + } __add_offloaded_reg_to_fifo(vub300, register_access, func); } @@ -1372,7 +1372,7 @@ static void download_offload_pseudocode(struct vub300_mmc_host *vub300) l += snprintf(vub300->vub_name + l, sizeof(vub300->vub_name) - l, "_%04X%04X", sf->vendor, sf->device); - }; + } snprintf(vub300->vub_name + l, sizeof(vub300->vub_name) - l, ".bin"); dev_info(&vub300->udev->dev, "requesting offload firmware %s\n", vub300->vub_name); @@ -1893,7 +1893,7 @@ static int satisfy_request_from_offloaded_data(struct vub300_mmc_host *vub300, i += 1; continue; } - }; + } if (vub300->total_offload_count == 0) return 0; else if (vub300->fn[func].offload_count == 0) -- 2.4.3 -- 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
[RESEND PATCH] usb: phy: isp1301: Export I2C module alias information
The I2C core always reports the MODALIAS uevent as "i2c:
---
Hello,
This is another resend of this patch. The first post was on July 30 [0]
as a part of a set and then the patch was resent again on August 25 [1].
It is a really trivial patch that fixes a bug (module auto loading not
working) so in case that already missed 4.3, I think it's -rc material.
Best regards,
Javier
[0]: https://lkml.org/lkml/2015/7/30/511
[1]: https://lkml.org/lkml/2015/8/25/40
drivers/usb/phy/phy-isp1301.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/phy/phy-isp1301.c b/drivers/usb/phy/phy-isp1301.c
index 8a55b37d1a02..db68156568e6 100644
--- a/drivers/usb/phy/phy-isp1301.c
+++ b/drivers/usb/phy/phy-isp1301.c
@@ -31,6 +31,7 @@ static const struct i2c_device_id isp1301_id[] = {
{ "isp1301", 0 },
{ }
};
+MODULE_DEVICE_TABLE(i2c, isp1301_id);
static struct i2c_client *isp1301_i2c_client;
--
2.4.3
--
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: [RESEND PATCH 3/7] usb: phy: isp1301: Export I2C module alias information
Hello,
On 09/01/2015 05:29 PM, Javier Martinez Canillas wrote:
> Hello Greg and Felipe,
>
> On 08/25/2015 08:31 AM, Javier Martinez Canillas wrote:
>> The I2C core always reports the MODALIAS uevent as "i2c:> regardless if the driver was matched using the I2C id_table or the
>> of_match_table. So the driver needs to export the I2C table and this
>> be built into the module or udev won't have the necessary information
>> to auto load the correct module when the device is added.
>>
>> Signed-off-by: Javier Martinez Canillas
>>
>> ---
>>
>> drivers/usb/phy/phy-isp1301.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/usb/phy/phy-isp1301.c b/drivers/usb/phy/phy-isp1301.c
>> index 8a55b37d1a02..db68156568e6 100644
>> --- a/drivers/usb/phy/phy-isp1301.c
>> +++ b/drivers/usb/phy/phy-isp1301.c
>> @@ -31,6 +31,7 @@ static const struct i2c_device_id isp1301_id[] = {
>> { "isp1301", 0 },
>> { }
>> };
>> +MODULE_DEVICE_TABLE(i2c, isp1301_id);
>>
>> static struct i2c_client *isp1301_i2c_client;
>>
>>
>
> Any comments about this patch? The first version
> was posted about a month ago and is very trivial.
>
Another gentle ping about this patch.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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: [RESEND PATCH 3/7] usb: phy: isp1301: Export I2C module alias information
Hello Greg and Felipe,
On 08/25/2015 08:31 AM, Javier Martinez Canillas wrote:
> The I2C core always reports the MODALIAS uevent as "i2c: regardless if the driver was matched using the I2C id_table or the
> of_match_table. So the driver needs to export the I2C table and this
> be built into the module or udev won't have the necessary information
> to auto load the correct module when the device is added.
>
> Signed-off-by: Javier Martinez Canillas
>
> ---
>
> drivers/usb/phy/phy-isp1301.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/phy/phy-isp1301.c b/drivers/usb/phy/phy-isp1301.c
> index 8a55b37d1a02..db68156568e6 100644
> --- a/drivers/usb/phy/phy-isp1301.c
> +++ b/drivers/usb/phy/phy-isp1301.c
> @@ -31,6 +31,7 @@ static const struct i2c_device_id isp1301_id[] = {
> { "isp1301", 0 },
> { }
> };
> +MODULE_DEVICE_TABLE(i2c, isp1301_id);
>
> static struct i2c_client *isp1301_i2c_client;
>
>
Any comments about this patch? The first version
was posted about a month ago and is very trivial.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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
[RESEND PATCH 0/7] Export I2C and OF module aliases in missing drivers
Hello, This is a resend of the patches that were not picked from the series "[PATCH 00/27] Export I2C and OF module aliases in missing drivers" [0] posted about a month ago. The patches have no dependencies and can be picked individually by the relevant maintainer. I preferred to resend instead of sending a naked ping for each patch that I got no answer. Best regards, Javier Javier Martinez Canillas (7): i2c: core: Export I2C module alias information in dummy driver backlight: tosa: Export I2C module alias information usb: phy: isp1301: Export I2C module alias information ALSA: ppc: keywest: Export I2C module alias information extcon: Export OF module alias information in missing drivers leds: Export OF module alias information in missing drivers regulator: isl9305: Export OF module alias information drivers/extcon/extcon-rt8973a.c | 1 + drivers/extcon/extcon-sm5502.c| 1 + drivers/i2c/i2c-core.c| 1 + drivers/leds/leds-pca963x.c | 1 + drivers/leds/leds-tca6507.c | 1 + drivers/regulator/isl9305.c | 1 + drivers/usb/phy/phy-isp1301.c | 1 + drivers/video/backlight/tosa_bl.c | 1 + sound/ppc/keywest.c | 1 + 9 files changed, 9 insertions(+) -- 2.4.3 -- 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
[RESEND PATCH 3/7] usb: phy: isp1301: Export I2C module alias information
The I2C core always reports the MODALIAS uevent as "i2c:
---
drivers/usb/phy/phy-isp1301.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/phy/phy-isp1301.c b/drivers/usb/phy/phy-isp1301.c
index 8a55b37d1a02..db68156568e6 100644
--- a/drivers/usb/phy/phy-isp1301.c
+++ b/drivers/usb/phy/phy-isp1301.c
@@ -31,6 +31,7 @@ static const struct i2c_device_id isp1301_id[] = {
{ "isp1301", 0 },
{ }
};
+MODULE_DEVICE_TABLE(i2c, isp1301_id);
static struct i2c_client *isp1301_i2c_client;
--
2.4.3
--
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 2/2] usb: Add support for ACPI identification to xhci-platform
Hello Greg,
On Sat, Aug 8, 2015 at 5:37 PM, Greg KH wrote:
> On Sat, Aug 08, 2015 at 07:43:40AM +0200, Javier Martinez Canillas wrote:
>> Hello Greg,
>>
>> On Sat, Aug 8, 2015 at 3:29 AM, Greg KH wrote:
>> > On Fri, Aug 07, 2015 at 06:03:36PM -0700, Duc Dang wrote:
>> >> Provide the methods to let ACPI identify the need to use
>> >> xhci-platform. Change the Kconfig files so the
>> >> xhci-plat.o file is selectable during kernel config.
>> >>
>> >> This has been tested on an ARM64 machine with platform XHCI, an
>> >> x86_64 machine with XHCI, and an x86_64 machine without XHCI.
>> >> There were no regressions or error messages on the machines
>> >> without platform XHCI.
>> >>
>> >> [dhdang: regenerate the patch over 4.2-rc5]
>> >> Signed-off-by: Mark Langsdorf
>> >> Signed-off-by: Duc Dang
>> >>
>> >> ---
>> >> Changes from v3:
>> >> Regenerate the patch over 4.2-rc5
>> >> No code change
>> >>
>> >> Changes from v2
>> >> Replaced tristate with a boolean as the driver doesn't
>> >> compile as a module
>> >> Correct --help-- to ---help---
>> >>
>> >> Changes from v1
>> >> Renamed from "add support for APM X-Gene to xhci-platform"
>> >> Removed changes to arm64/Kconfig
>> >> Made CONFIG_USB_XHCI_PLATFORM a user selectable config option
>> >>
>> >> drivers/usb/host/Kconfig | 7 ++-
>> >> drivers/usb/host/xhci-plat.c | 11 +++
>> >> 2 files changed, 17 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>> >> index 8afc3c1..96231ee 100644
>> >> --- a/drivers/usb/host/Kconfig
>> >> +++ b/drivers/usb/host/Kconfig
>> >> @@ -32,7 +32,12 @@ config USB_XHCI_PCI
>> >> default y
>> >>
>> >> config USB_XHCI_PLATFORM
>> >> - tristate
>> >> + tristate "xHCI platform driver support"
>> >> + ---help---
>> >> + Say 'Y' to enable the support for the xHCI host controller
>> >> + as a platform device. Many ARM SoCs provide USB this way.
>> >> +
>> >> + If unsure, say 'Y'.
>> >>
>> >> config USB_XHCI_MVEBU
>> >> tristate "xHCI support for Marvell Armada 375/38x"
>> >> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> >> index 5d03f8b..14b40d2 100644
>> >> --- a/drivers/usb/host/xhci-plat.c
>> >> +++ b/drivers/usb/host/xhci-plat.c
>> >> @@ -19,6 +19,7 @@
>> >> #include
>> >> #include
>> >> #include
>> >> +#include
>> >>
>> >> #include "xhci.h"
>> >> #include "xhci-mvebu.h"
>> >> @@ -262,6 +263,15 @@ static const struct of_device_id usb_xhci_of_match[]
>> >> = {
>> >> MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
>> >> #endif
>> >>
>> >> +#ifdef CONFIG_ACPI
>> >
>> > You shoudn't need this #ifdef, right?
>> >
>>
>> Why it is not needed?
>
> Why is it needed?
>
As explained, to have avoid having an unused variable.
>> The driver does .acpi_match_table = ACPI_PTR(usb_xhci_acpi_match) and
>> ACPI_PTR() is NULL if CONFIG_ACPI is not enabled. Which can happen
>> AFAIU since the driver also supports OF. So without the #ifdef guards,
>> .acpi_match_table = NULL and the struct acpi_device_id
>> usb_xhci_acpi_match[] will be built but not used.
>
> Which is just fine, right?
>
I've seen people having different opinions about this specific case
(using #ifdef guards for ACPI, OF, etc match tables definition),
that's why I asked.
>> Or am I missing something?
>
> Don't put #ifdef in .c files if at all possible is the kernel style
> rules.
>
I know but as you said the rule is to not have #ifdef if possible. But
I understand now that for you this case doesn't justify the #ifdefery.
> thanks,
>
> greg k-h
Best regards,
Javier
--
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 2/2] usb: Add support for ACPI identification to xhci-platform
Hello Greg,
On Sat, Aug 8, 2015 at 3:29 AM, Greg KH wrote:
> On Fri, Aug 07, 2015 at 06:03:36PM -0700, Duc Dang wrote:
>> Provide the methods to let ACPI identify the need to use
>> xhci-platform. Change the Kconfig files so the
>> xhci-plat.o file is selectable during kernel config.
>>
>> This has been tested on an ARM64 machine with platform XHCI, an
>> x86_64 machine with XHCI, and an x86_64 machine without XHCI.
>> There were no regressions or error messages on the machines
>> without platform XHCI.
>>
>> [dhdang: regenerate the patch over 4.2-rc5]
>> Signed-off-by: Mark Langsdorf
>> Signed-off-by: Duc Dang
>>
>> ---
>> Changes from v3:
>> Regenerate the patch over 4.2-rc5
>> No code change
>>
>> Changes from v2
>> Replaced tristate with a boolean as the driver doesn't
>> compile as a module
>> Correct --help-- to ---help---
>>
>> Changes from v1
>> Renamed from "add support for APM X-Gene to xhci-platform"
>> Removed changes to arm64/Kconfig
>> Made CONFIG_USB_XHCI_PLATFORM a user selectable config option
>>
>> drivers/usb/host/Kconfig | 7 ++-
>> drivers/usb/host/xhci-plat.c | 11 +++
>> 2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>> index 8afc3c1..96231ee 100644
>> --- a/drivers/usb/host/Kconfig
>> +++ b/drivers/usb/host/Kconfig
>> @@ -32,7 +32,12 @@ config USB_XHCI_PCI
>> default y
>>
>> config USB_XHCI_PLATFORM
>> - tristate
>> + tristate "xHCI platform driver support"
>> + ---help---
>> + Say 'Y' to enable the support for the xHCI host controller
>> + as a platform device. Many ARM SoCs provide USB this way.
>> +
>> + If unsure, say 'Y'.
>>
>> config USB_XHCI_MVEBU
>> tristate "xHCI support for Marvell Armada 375/38x"
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index 5d03f8b..14b40d2 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -19,6 +19,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include "xhci.h"
>> #include "xhci-mvebu.h"
>> @@ -262,6 +263,15 @@ static const struct of_device_id usb_xhci_of_match[] = {
>> MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
>> #endif
>>
>> +#ifdef CONFIG_ACPI
>
> You shoudn't need this #ifdef, right?
>
Why it is not needed?
The driver does .acpi_match_table = ACPI_PTR(usb_xhci_acpi_match) and
ACPI_PTR() is NULL if CONFIG_ACPI is not enabled. Which can happen
AFAIU since the driver also supports OF. So without the #ifdef guards,
.acpi_match_table = NULL and the struct acpi_device_id
usb_xhci_acpi_match[] will be built but not used.
Or am I missing something?
> thanks,
>
> greg k-h
>
Best regards,
Javier
--
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/27] Export I2C and OF module aliases in missing drivers
Hello Dmitry, Thanks a lot for your feedback. On 07/30/2015 06:37 PM, Dmitry Torokhov wrote: > On Thu, Jul 30, 2015 at 09:35:17AM -0700, Dmitry Torokhov wrote: >> Hi Javier, >> >> On Thu, Jul 30, 2015 at 06:18:25PM +0200, Javier Martinez Canillas wrote: >>> Hello, >>> >>> Short version: >>> >>> This series add the missing MODULE_DEVICE_TABLE() for OF and I2C tables >>> to export that information so modules have the correct aliases built-in >>> and autoloading works correctly. >>> >>> Longer version: >>> >>> Currently it's mandatory for I2C drivers to have an I2C device ID table >>> regardless if the device was registered using platform data or OF. This >>> is because the I2C core needs an I2C device ID table for two reasons: >>> >>> 1) Match the I2C client with a I2C device ID so a struct i2c_device_id >>>is passed to the I2C driver probe() function. >>> >>> 2) Export the module aliases from the I2C device ID table so userspace >>>can auto-load the correct module. This is because i2c_device_uevent >>>always reports a MODALIAS of the form i2c:name>. >> >> Why are we not fixing this? We emit specially carved uevent for >> ACPI-based devices, why not the same for OF? Platform bus does this... > > Ah, now I see the 27/27 patch. I think it is exactly what we need. And Yes, patch 27/27 is needed but the problem is as I explained before that there are drivers relying on the current behavior. The item c) in the list of issues that I mentioned. So those drivers need to be fixed before that patch is merged... > probably for SPI bus as well. > Yes, I didn't mention SPI because the cover letter became too long already but it does indeed have the same issue and I discussed this with Mark already some time ago [0]. Once I2C uevent report is fixed, I plan to do the same for SPI. > Thanks. > [0]: https://lkml.org/lkml/2014/9/11/458 Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- 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 12/27] usb: phy: isp1301: Export I2C module alias information
The I2C core always reports the MODALIAS uevent as "i2c:
---
drivers/usb/phy/phy-isp1301.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/phy/phy-isp1301.c b/drivers/usb/phy/phy-isp1301.c
index 8a55b37d1a02..db68156568e6 100644
--- a/drivers/usb/phy/phy-isp1301.c
+++ b/drivers/usb/phy/phy-isp1301.c
@@ -31,6 +31,7 @@ static const struct i2c_device_id isp1301_id[] = {
{ "isp1301", 0 },
{ }
};
+MODULE_DEVICE_TABLE(i2c, isp1301_id);
static struct i2c_client *isp1301_i2c_client;
--
2.4.3
--
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 00/27] Export I2C and OF module aliases in missing drivers
Hello, Short version: This series add the missing MODULE_DEVICE_TABLE() for OF and I2C tables to export that information so modules have the correct aliases built-in and autoloading works correctly. Longer version: Currently it's mandatory for I2C drivers to have an I2C device ID table regardless if the device was registered using platform data or OF. This is because the I2C core needs an I2C device ID table for two reasons: 1) Match the I2C client with a I2C device ID so a struct i2c_device_id is passed to the I2C driver probe() function. 2) Export the module aliases from the I2C device ID table so userspace can auto-load the correct module. This is because i2c_device_uevent always reports a MODALIAS of the form i2c:name>. Lee Jones posted a patch series [0] to solve 1) by allowing the I2C drivers to have a probe() function that does not get a i2c_device_id. The problem is that his series didn't take into account 2) so if that was merged and the I2C ID table is removed from all the drivers that don't needed it, module auto-loading will break for those. But even now there are many I2C drivers were module auto-loading is not working because of the fact that the I2C core always reports the MODALIAS as i2c:name> and many developers didn't expect this. I've identified I2C drivers with 3 types of different issues: a) Those that have an i2c_table but are not exported. The match works and the correct i2c_device_id is passed on probe but since the ID table is not exported, module auto-load won't work. b) Those that have a of_table but are not exported. This is currently not an issue since even when the of_table is used to match the dev with the driver, an OF modalias is not reported by the I2C core. But if the I2C core is changed to report the MODALIAS of the form of:N*T*C as it's made by other subsystems, then module auto-load will break for these drivers. c) Those that don't have a of_table but should since are OF drivers with DT bindings doc for them. Since the I2C core does not report a OF modalias and since i2c_device_match() fallbacks to match the device part of the compatible string with the I2C device ID table, many OF drivers don't have an of_table to match. After all having a I2C device ID table is mandatory so it works without a of_table. So, in order to not make mandatory to have a I2C device ID table, at least a) and b) needs to be addressed, this series does that. c) should be fixed too since it seems wrong that a driver with a DT binding document, does not have a OF table and export it to modules. Also stripping the vendor part from the compatible string to match with the I2C devices ID table and reporting only the device name to user-space doesn't seem to be correct. I've identified at least two drivers that have the same name on their I2C device ID table so the manufacturer prefix is important. But I've not tried to fix c) yet since that is not so easy to automate due drivers not having all the information (i.e: the device name can match a documented compatible string device part but without the vendor prefix is hard to tell). I split the changes so the patches in this series are independent and can be picked individually by subsystem maintainers. Patch #27 changes the logic of i2c_device_uevent() to report an OF modalias if the device was registered using OF. But this patch is included in the series only as an RFC for illustration purposes since changing that without fixing c) will break module auto-loading for the drivers of devices registered with OF but that don't have a of_match_table. Although arguably, those drivers were relying on the assumption that a MODALIAS=i2c: would always be reported even for the OF case which is not the true on other subsystems. [0]: https://lkml.org/lkml/2014/8/28/283 Best regards, Javier Javier Martinez Canillas (27): mfd: stw481x: Export I2C module alias information spi: xcomm: Export I2C module alias information iio: Export I2C module alias information in missing drivers [media] Export I2C module alias information in missing drivers macintosh: therm_windtunnel: Export I2C module alias information misc: eeprom: Export I2C module alias information in missing drivers Input: Export I2C module alias information in missing drivers power: Export I2C module alias information in missing drivers i2c: core: Export I2C module alias information in dummy driver backlight: tosa: Export I2C module alias information [media] staging: media: lirc: Export I2C module alias information usb: phy: isp1301: Export I2C module alias information ALSA: ppc: keywest: Export I2C module alias information hwmon: (nct7904) Export I2C module alias information regulator: fan53555: Export I2C module alias information mfd: Export OF module alias information in missing drivers iio: Export OF module alias information in miss
[PATCH 27/27] i2c: (RFC, don't apply) report OF style modalias when probing using DT
An I2C driver that supports both OF and legacy platforms, will have both a OF and I2C ID table. This means that when built as a module, the aliases will be filled from both tables but currently always an alias of the form i2c: is reported, e.g: $ cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias i2c:maxtouch So if a device is probed by matching its compatible string, udev can get a MODALIAS uevent env var that doesn't match with one of the valid aliases so the module won't be auto-loaded. This patch changes the I2C core to report a OF related MODALIAS uevent (of:N*T*C) env var instead so the module can be auto-loaded and also report the correct alias using sysfs: $ cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias of:NtrackpadTCatmel,maxtouch Signed-off-by: Javier Martinez Canillas --- drivers/i2c/i2c-core.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 92dddfeb3f39..c0668c2ed9da 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -489,6 +489,10 @@ static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env) struct i2c_client *client = to_i2c_client(dev); int rc; + rc = of_device_uevent_modalias(dev, env); + if (rc != -ENODEV) + return rc; + rc = acpi_device_uevent_modalias(dev, env); if (rc != -ENODEV) return rc; @@ -726,6 +730,10 @@ show_modalias(struct device *dev, struct device_attribute *attr, char *buf) struct i2c_client *client = to_i2c_client(dev); int len; + len = of_device_get_modalias(dev, buf, PAGE_SIZE - 1); + if (len != -ENODEV) + return len; + len = acpi_device_modalias(dev, buf, PAGE_SIZE -1); if (len != -ENODEV) return len; -- 2.4.3 -- 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: [RESEND 1/2] usb: ehci-exynos: Make provision for vdd regulators
Hello Krzysztof, On Mon, Jun 8, 2015 at 8:52 AM, Krzysztof Kozlowski wrote: > 2015-06-08 15:42 GMT+09:00 Javier Martinez Canillas : >> Hello, >> >> On Mon, Jun 8, 2015 at 7:14 AM, Krzysztof Kozlowski >> wrote: >> >> [...] >> >>> >>> To summarize my point of view: >>> 1. Unless Vivek's says otherwise, please give him the credits with >>> proper "from" field. >>> 2. Issues mentioned in previous mail should be addressed (missing >>> IS_ERR(), how disabling the regulator during suspend affects waking >>> up). >>> 3. The patchset must be tested, even after rebasing. >>> >> >> Agreed with all the points. >> >> Anand, >> >> An easy way to preserve authorship when rebasing patches is to use the >> git command author option. As an example you can execute the following >> command: >> >> $ git commit -a -s --author='Vivek Gautam ' > > By default "git am" and "git format-patch" preserve the author of a > patch so usually this step is not necessary. Unless the patch is > applied in a different way... :) > That is correct but if an old patch still applies cleanly on top of current's tree, then there is no need to do a rebase right? ;-) I mean, git am is not as smart as the patch command for example to detect when the line numbers mentioned in the patch are incorrect and does not attempt to find the correct place to apply each hunk of the patch (at least by default, I don't know if there is an option). Which IIUC is what Anand had to do so in that case you need to manually commit again but using the original patch author. > Best regards, > Krzysztof Best regards, Javier -- 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: [RESEND 1/2] usb: ehci-exynos: Make provision for vdd regulators
Hello, On Mon, Jun 8, 2015 at 7:14 AM, Krzysztof Kozlowski wrote: [...] > > To summarize my point of view: > 1. Unless Vivek's says otherwise, please give him the credits with > proper "from" field. > 2. Issues mentioned in previous mail should be addressed (missing > IS_ERR(), how disabling the regulator during suspend affects waking > up). > 3. The patchset must be tested, even after rebasing. > Agreed with all the points. Anand, An easy way to preserve authorship when rebasing patches is to use the git command author option. As an example you can execute the following command: $ git commit -a -s --author='Vivek Gautam ' > Best regards, > Krzysztof Best regards, Javier -- 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/2] arm: omap: remove *.auto* from device names given in usb_bind_phy
Hi Kishon,
On Mon, Dec 9, 2013 at 7:07 AM, Kishon Vijay Abraham I wrote:
> Hi,
>
>
> On Saturday 07 December 2013 02:38 AM, Felipe Balbi wrote:
>>
>> Hi,
>>
>> On Fri, Dec 06, 2013 at 01:14:38PM +0100, Javier Martinez Canillas wrote:
>>>
>>> On Fri, Dec 6, 2013 at 1:06 PM, Kishon Vijay Abraham I
>>> wrote:
>>>>
>>>> Previously MUSB wrapper (OMAP) device used PLATFORM_DEVID_AUTO while
>>>> creating
>>>> MUSB core device. So in usb_bind_phy (binds the controller with the
>>>> PHY), the
>>>> device name of the controller had *.auto* in it. Since with using
>>>> PLATFORM_DEVID_AUTO, there is no way to know the exact device name in
>>>> advance,
>>>> the data given in usb_bind_phy became obsolete and usb_get_phy was
>>>> failing.
>>>> So MUSB wrapper was modified not to use PLATFORM_DEVID_AUTO.
>>>> Corresponding
>>>> change is done in board file here.
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I
>>>> ---
>>>> arch/arm/mach-omap2/board-2430sdp.c|2 +-
>>>> arch/arm/mach-omap2/board-3430sdp.c|2 +-
>>>> arch/arm/mach-omap2/board-cm-t35.c |2 +-
>>>> arch/arm/mach-omap2/board-devkit8000.c |2 +-
>>>> arch/arm/mach-omap2/board-ldp.c|2 +-
>>>> arch/arm/mach-omap2/board-omap3beagle.c|2 +-
>>>> arch/arm/mach-omap2/board-omap3logic.c |2 +-
>>>> arch/arm/mach-omap2/board-omap3pandora.c |2 +-
>>>> arch/arm/mach-omap2/board-omap3stalker.c |2 +-
>>>> arch/arm/mach-omap2/board-omap3touchbook.c |2 +-
>>>> arch/arm/mach-omap2/board-overo.c |2 +-
>>>> arch/arm/mach-omap2/board-rx51.c |2 +-
>>>> 12 files changed, 12 insertions(+), 12 deletions(-)
>>>>
>>>
>>> You can drop this patch since boards files are being removed for v3.14
>>
>>
>> if we can drop this patch, the whole series is invalid, since we'll be
>> using DT phandles to find PHYs going forward, no ?
>>
> yeah. But in one of the other threads, Tony seemed ok to take a patch that
> fixes the same issue in mach-omap2/twl-common.c. So it's better to confirm
> with Tony.
>
Yes, I just read the other thread ("[PATCH] omap: twl-common: Fix
musb-hdrc device name") and I see that these patches are fixing a
v3.13 regression and are meant for the -rc cycle and not for v3.14.
Sorry for the noise then.
Best regards,
Javier
> Thanks
> Kishon
--
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/2] arm: omap: remove *.auto* from device names given in usb_bind_phy
Hi Kishon, On Fri, Dec 6, 2013 at 1:06 PM, Kishon Vijay Abraham I wrote: > Previously MUSB wrapper (OMAP) device used PLATFORM_DEVID_AUTO while creating > MUSB core device. So in usb_bind_phy (binds the controller with the PHY), the > device name of the controller had *.auto* in it. Since with using > PLATFORM_DEVID_AUTO, there is no way to know the exact device name in advance, > the data given in usb_bind_phy became obsolete and usb_get_phy was failing. > So MUSB wrapper was modified not to use PLATFORM_DEVID_AUTO. Corresponding > change is done in board file here. > > Signed-off-by: Kishon Vijay Abraham I > --- > arch/arm/mach-omap2/board-2430sdp.c|2 +- > arch/arm/mach-omap2/board-3430sdp.c|2 +- > arch/arm/mach-omap2/board-cm-t35.c |2 +- > arch/arm/mach-omap2/board-devkit8000.c |2 +- > arch/arm/mach-omap2/board-ldp.c|2 +- > arch/arm/mach-omap2/board-omap3beagle.c|2 +- > arch/arm/mach-omap2/board-omap3logic.c |2 +- > arch/arm/mach-omap2/board-omap3pandora.c |2 +- > arch/arm/mach-omap2/board-omap3stalker.c |2 +- > arch/arm/mach-omap2/board-omap3touchbook.c |2 +- > arch/arm/mach-omap2/board-overo.c |2 +- > arch/arm/mach-omap2/board-rx51.c |2 +- > 12 files changed, 12 insertions(+), 12 deletions(-) > You can drop this patch since boards files are being removed for v3.14 Thanks a lot and best regards, Javier -- 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/4] usb: musb: Call atomic_notifier_call_chain when status is changed
On Wed, Sep 18, 2013 at 4:22 PM, Pavel Machek wrote: > Hi! > >> >> >> > So will you do that? Or it is needed to resend this one line >> >> >> > hunk again in new email again? >> >> >> >> >> >> new patch, new email >> >> > >> >> > Guys, WHY ARE YOU SO STUPID AND ARROGANT? >> >> > >> >> > Sorry but, need to copy full isolated patch/hunk from one mail to >> >> > another is hassling. So what you want from me? Do all those non >> >> > sense working only because yesterday you had bad day? Or what? > ... >> > Actually, there is need to be rude, because Felipe fails to act as >> > maintainer. Instead of fixing bugs in his code, he bounces bugfix >> > patches, points people to random READMEs and wastes everyones time. >> >> I don't know what are you talking about (if that happened in another >> thread then I need more context). Felipe is not bouncing any bugfix > > Take a look here: > > https://lkml.org/lkml/2013/9/17/286 > > I clearly state that patch can not be tested as required for "proper" > submission, but offer patch anyway. I get irrelevant boilerplate on > patch format. > Felipe didn't complain about you not being to be able to test the patch (most of the times compile tested if enough) what he said was: "Seriously though, read that file, you're commit log has garbage in it which shouldn't go to git history". which is totally true, if you want to comment things that don't have to go to the backlog you can't comment between the --- after your s-o-b and before the first diff. That's were you should puts comments like "Hi! or Here's suggested patch. I don't have the hardware, so it is completely untested." >> but just asked to split the patch in two since the patch was solving >> two separate issues so is way better to have it in two separate >> patches for the reasons I explained before. >> >> So, as far as I can tell Felipe did exactly what I would expect from a >> maintainer. He took the time to review the patches sent to him and > > I'd expect maintainer to, well, maintain code. It means actually > fixing bugs in his code, when he's pointed at them. > >> gave feedback. If the sender doesn't want to take his feedback into >> account and prefer to send pretty insulting emails instead that is his >> choice but I would say that is this not the greatest approach to get >> your code merged (to say the least). > > Clearly not. But Pali found bug in code Felipe should > maintain. Instead of "thank you for bug report, I applied this one > line of your code to fix it", Pali got "new patch, new email" for his > efforts. That is how you train dogs, not how you should treat kernel > contributors. > No, you misunderstood the role of the maintainers. Maintainers should be custodian of a part of the kernel but not responsible for every single line of code on their sub-systems. If a piece of code is buggy then the people using and that take care of that should be fixing and maintainers should review and suggest improvements to the patches. As long as a piece of code keep compiling then it is harmless even if is buggy and nobody cares about it. If it is so broken that it doesn't even compile then the maintainer can decide to just drop it since no one else seems to care about it. If someone finds a bug on a piece of code is because that people care about that functionality. Maintainers are really busy people and can jump and fix any random bug that someone finds on a piece of code just because it is the subsystem they maintainer neither they have to blindly merge any crappy patch just because they don't have time (or interest) in fixing a reported bug on a piece of code. > Now, it is possible that Felipe just has problems with english, as he > called me piece of wood in https://lkml.org/lkml/2013/9/17/476 , but > he appears more arogant than usual over email. > Clearly he meant "your commit log has garbage" instead of you're, that's a typo. > Pavel But neither Felipe needs someone to defend him nor I have time to keep arguing with you. Have nice day! Javier -- 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/4] usb: musb: Call atomic_notifier_call_chain when status is changed
On Wed, Sep 18, 2013 at 3:30 PM, Pavel Machek wrote: > Hi! > >> >> > So will you do that? Or it is needed to resend this one line >> >> > hunk again in new email again? >> >> >> >> new patch, new email >> > >> > Guys, WHY ARE YOU SO STUPID AND ARROGANT? >> > >> > Sorry but, need to copy full isolated patch/hunk from one mail to >> > another is hassling. So what you want from me? Do all those non >> > sense working only because yesterday you had bad day? Or what? > ... >> >> Hi Pali, >> >> There is no need to be rude. >> >> Felipe asked you to do the split since he believes that the notifier >> chain call for musb xceiv and the twl->phy notifier head init should >> be done in two separate patches. > > Actually, there is need to be rude, because Felipe fails to act as > maintainer. Instead of fixing bugs in his code, he bounces bugfix > patches, points people to random READMEs and wastes everyones time. > Pavel > I don't know what are you talking about (if that happened in another thread then I need more context). Felipe is not bouncing any bugfix but just asked to split the patch in two since the patch was solving two separate issues so is way better to have it in two separate patches for the reasons I explained before. So, as far as I can tell Felipe did exactly what I would expect from a maintainer. He took the time to review the patches sent to him and gave feedback. If the sender doesn't want to take his feedback into account and prefer to send pretty insulting emails instead that is his choice but I would say that is this not the greatest approach to get your code merged (to say the least). Best regards, Javier -- 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/4] usb: musb: Call atomic_notifier_call_chain when status is changed
On Wed, Sep 18, 2013 at 10:20 AM, Pali Rohár wrote:
> On Wednesday 18 September 2013 03:49:42 Felipe Balbi wrote:
>> On Tue, Sep 17, 2013 at 09:28:42PM +0200, Pali Rohár wrote:
>> > On Tuesday 17 September 2013 18:08:35 Felipe Balbi wrote:
>> > > On Tue, Sep 17, 2013 at 06:05:15PM +0200, Pali Rohár wrote:
>> > > > On Tuesday 17 September 2013 17:48:59 you wrote:
>> > > > > On Sun, Sep 08, 2013 at 10:50:36AM +0200, Pali Rohár wrote:
>> > > > > > More power supply drivers depends on vbus events and
>> > > > > > without it they not working. Power supply drivers
>> > > > > > using usb_register_notifier, so to deliver events
>> > > > > > it is needed to call atomic_notifier_call_chain.
>> > > > > >
>> > > > > > So without atomic notifier power supply driver
>> > > > > > isp1704 not retrieving vbus status and reporting
>> > > > > > bogus values to userspace and also to board
>> > > > > > platform data functions. Without proper data
>> > > > > > charger drivers trying to charge battery also when
>> > > > > > charger is disconnected or do not start charging
>> > > > > > when wallcharger connects.
>> > > > > >
>> > > > > > Atomic notifier in musb driver was used before v3.5
>> > > > > > and was replaced with omap mailbox. This patch
>> > > > > > adding atomic_notifier_call_chain call from
>> > > > > > function omap_musb_set_mailbox.
>> > > > > >
>> > > > > > Signed-off-by: Pali Rohár
>> > > > > > ---
>> > > > > >
>> > > > > > drivers/usb/musb/omap2430.c |3 +++
>> > > > > > drivers/usb/phy/phy-twl4030-usb.c |2 ++
>> > > > > > 2 files changed, 5 insertions(+)
>> > > > > >
>> > > > > > diff --git a/drivers/usb/musb/omap2430.c
>> > > > > > b/drivers/usb/musb/omap2430.c index f44e8b5..5c40252
>> > > > > > 100644 --- a/drivers/usb/musb/omap2430.c
>> > > > > > +++ b/drivers/usb/musb/omap2430.c
>> > > > > > @@ -305,6 +305,9 @@ static void
>> > > > > > omap_musb_set_mailbox(struct omap2430_glue *glue)
>> > > > > >
>> > > > > > default:
>> > > > > > dev_dbg(dev, "ID float\n");
>> > > > > >
>> > > > > > }
>> > > > > >
>> > > > > > +
>> > > > > > + atomic_notifier_call_chain(&musb->xceiv->notifier,
>> > > > > > + musb->xceiv->last_event, NULL);
>> > > > >
>> > > > > let's add a wrapper for this:
>> > > > >
>> > > > > static inline int usb_phy_notify(struct usb phy *x,
>> > > > > unsigned val, void *v) {
>> > > > >
>> > > > > return atomic_notifier_call_chain(&x->notifier, val,
>> > > > > v);
>> > > > >
>> > > > > }
>> > > >
>> > > > Where to add this wrapper? To omap2430.c? or some
>> > > > include file?
>> > >
>> > >
>> > >
>> > > > On Tuesday 17 September 2013 17:49:17 Felipe Balbi wrote:
>> > > > > On Sun, Sep 08, 2013 at 10:50:36AM +0200, Pali Rohár wrote:
>> > > > > > diff --git a/drivers/usb/phy/phy-twl4030-usb.c
>> > > > > > b/drivers/usb/phy/phy-twl4030-usb.c index
>> > > > > > 8f78d2d..efe6155 100644
>> > > > > > --- a/drivers/usb/phy/phy-twl4030-usb.c
>> > > > > > +++ b/drivers/usb/phy/phy-twl4030-usb.c
>> > > > > > @@ -705,6 +705,8 @@ static int
>> > > > > > twl4030_usb_probe(struct platform_device *pdev)
>> > > > > >
>> > > > > > if (device_create_file(&pdev->dev,
>> > > > > > &dev_attr_vbus))
>> > > > > >
>> > > > > > dev_warn(&pdev->dev, "could not create sysfs
>> > > > > > file\n");
>> > > > > >
>> > > > > > + ATOMIC_INIT_NOTIFIER_HEAD(&twl->phy.notifier);
>> > > > >
>> > > > > BTW, this is a bugfix, send separately.
>> > > >
>> > > > What to send separately?
>> > > >
>> > > > This full patch 1/4 is bugfix. And I did not understand
>> > > > what you want. Maybe it could be easier for you to
>> > > > apply this small 3+2 lines patch how you need.
>> > >
>> > > This hunk here (initializaing notifier head) is a separate
>> > > bug fix and needs its own patch.
>> >
>> > So will you do that? Or it is needed to resend this one line
>> > hunk again in new email again?
>>
>> new patch, new email
>
> Guys, WHY ARE YOU SO STUPID AND ARROGANT?
>
> Sorry but, need to copy full isolated patch/hunk from one mail to
> another is hassling. So what you want from me? Do all those non
> sense working only because yesterday you had bad day? Or what?
>
> Everything needed with described information was in first mail.
> Also second hunk has full isolated "git diff" output, so it is for
> you really big problem to copy it? Or you did not see that patch?
>
> I really did not understand what you wanted from me.
>
>
> BEGINNING OF PATCH
>
>
> This is bugfix and sending this patch separately from all other patches.
> This patch is visibly isolated from all others and could be readable also
> by disabled people. For other handicapped people I suggest to increase
> font size and other text settings in program which view this patch.
> For visually impaired people I suggest to use some text-to-speech software.
>
> This is small 2 lines patch, diff starting after next visi
Re: [PATCH 1/1] usb: host: xhci: remove unused trb var in xhci_irq()
On 12/17/2012 11:40 PM, Sarah Sharp wrote: > Hi Javier, > > Thanks for catching this! The patch is going to have to wait until > after the 3.8 merge window closes, since I believe Greg isn't accepting > usb-next pull requests yet. > > Sarah Sharp > > On Sun, Dec 16, 2012 at 04:21:31AM +0100, Javier Martinez Canillas wrote: Hi Sarah, Ok, no worries. Thanks a lot and best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] usb: host: xhci: remove unused trb var in xhci_irq()
The union xhci_trb *trb variable is defined and assigned
inside the xHCI IRQ handler function but is never used.
Signed-off-by: Javier Martinez Canillas
---
drivers/usb/host/xhci-ring.c |2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index cbb44b7..b1b6abb 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2697,13 +2697,11 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd)
{
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
u32 status;
- union xhci_trb *trb;
u64 temp_64;
union xhci_trb *event_ring_deq;
dma_addr_t deq;
spin_lock(&xhci->lock);
- trb = xhci->event_ring->dequeue;
/* Check if the xHC generated the interrupt, or the irq is shared */
status = xhci_readl(xhci, &xhci->op_regs->status);
if (status == 0x)
--
1.7.7.6
--
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: issue with uvcvideo and xhci
On 12/12/2012 06:52 PM, Sarah Sharp wrote: > On Wed, Dec 12, 2012 at 12:47:24PM +0100, Javier Martinez Canillas wrote: >> Hello, >> >> We have an issue when trying to use USB cameras on a particular machine using >> the latest mainline Linux 3.7 kernel. This is not a regression since the same >> issue is present with older kernels (i.e: 3.5). >> >> The cameras work fine when plugged to an USB2.0 port (using the EHCI HCD host >> controller driver) but they don't when using the USB3.0 port (using the xHCI >> HCD host controller driver). >> >> The machine's USB3.0 host controller is a NEC Corporation uPD720200 USB 3.0 >> Host >> Controller (rev 04). >> >> When enabling trace on the uvcvideo driver I see that most frames are lost: >> >> Dec 12 11:07:58 thinclient kernel: [ 4965.597637] uvcvideo: USB isochronous >> frame lost (-18). >> Dec 12 11:07:58 thinclient kernel: [ 4965.597642] uvcvideo: USB isochronous >> frame lost (-18). >> Dec 12 11:07:58 thinclient kernel: [ 4965.597647] uvcvideo: Marking buffer as >> bad (error bit set). >> Dec 12 11:07:58 thinclient kernel: [ 4965.597651] uvcvideo: Frame complete >> (EOF >> found). >> Dec 12 11:07:58 thinclient kernel: [ 4965.597655] uvcvideo: EOF in empty >> payload. >> Dec 12 11:07:58 thinclient kernel: [ 4965.597661] uvcvideo: Dropping payload >> (out of sync). >> Dec 12 11:07:58 thinclient kernel: [ 4965.813294] uvcvideo: frame 486 stats: >> 0/2/8 packets, 0/0/8 pts >> >> The uvcvideo checks if urb->iso_frame_desc[i].status < 0 on the >> uvc_video_decode_isoc() function (drivers/media/usb/uvc/uvc_video.c). >> >> I checked on the xhci driver and the only place where this error code >> (-EXDEV) >> is assigned to frame->status is inside the skip_isoc_td() function >> (drivers/usb/host/xhci-ring.c). >> >> At this point I'm not sure if this is a bug on the xhci driver, another quirk >> needed by the XHCI_NEC_HOST, a camera misconfiguration on the USB Video Class >> driver or a firmware/hardware bug. > > It's a known performance issue, although it's not clear whether it's on > the xHCI driver side or the host controller side. When an interface > setting is enabled where the isochronous endpoint requires two > additional transfers per service interval, the NEC host controller > starts reporting many missed service intervals. The xHCI driver then > finds all the frame buffers that were skipped and marks them with the > -EXDEV status. > > An error status of Missed Service Interval means the host controller > could not access the transfer memory fast enough through the PCI bus to > service the endpoint in time. It could be a host hardware issue, or it > could be software slowing down the system to a crawl. I lean towards a > software issue since, as you said, the Windows driver works fine. > (Although who knows what NEC quirks the Windows driver is working > around...) > > The NEC xHCI host controller is a 0.96 revision, which doesn't support > the Block Event Interrupt (BEI) flag which cuts down on the number of > interrupts per URB submitted. So the xHCI driver's interrupt routine > gets called on every single service interval, rather than being called > once per URB. > > Since the Linux xHCI driver isn't really optimized for performance yet, > the interrupt handler is probably pretty slow and could cause delays in > submitting future URBs. The high amount of interrupts is probably > causing other systems to be starved, possibly leading to the xHCI host > controller not being able to access memory fast enough to service the > endpoint. > Hi Sarah, Thanks for the explanation. Now it makes sense to me and I understand why it works when I decrease either the frame rate or the frame size below certain thresholds. >> The cameras are reported to work on the same machine but using another >> operating >> system (Windows). > > Windows probably uses Event Data TRBs to cut the interrupts down to one > per URB. It would take a major effort to make the xHCI driver use Event > Data TRBs. > >> I was wondering if you can give me some pointers on how to be sure what's the >> issue or if this rings any bells to you. > > I don't have time to work on performance issues right now, as I have > several other critical bugs (mostly around failed S3/S4). However, if > you want to try to fix this issue yourself, I suggest you run perf and > see where the bottle necks in the xHCI interrupt handler are. > > I suspect that part of it is that the interrupt handler reads the xHCI > status
issue with uvcvideo and xhci
Hello, We have an issue when trying to use USB cameras on a particular machine using the latest mainline Linux 3.7 kernel. This is not a regression since the same issue is present with older kernels (i.e: 3.5). The cameras work fine when plugged to an USB2.0 port (using the EHCI HCD host controller driver) but they don't when using the USB3.0 port (using the xHCI HCD host controller driver). The machine's USB3.0 host controller is a NEC Corporation uPD720200 USB 3.0 Host Controller (rev 04). When enabling trace on the uvcvideo driver I see that most frames are lost: Dec 12 11:07:58 thinclient kernel: [ 4965.597637] uvcvideo: USB isochronous frame lost (-18). Dec 12 11:07:58 thinclient kernel: [ 4965.597642] uvcvideo: USB isochronous frame lost (-18). Dec 12 11:07:58 thinclient kernel: [ 4965.597647] uvcvideo: Marking buffer as bad (error bit set). Dec 12 11:07:58 thinclient kernel: [ 4965.597651] uvcvideo: Frame complete (EOF found). Dec 12 11:07:58 thinclient kernel: [ 4965.597655] uvcvideo: EOF in empty payload. Dec 12 11:07:58 thinclient kernel: [ 4965.597661] uvcvideo: Dropping payload (out of sync). Dec 12 11:07:58 thinclient kernel: [ 4965.813294] uvcvideo: frame 486 stats: 0/2/8 packets, 0/0/8 pts The uvcvideo checks if urb->iso_frame_desc[i].status < 0 on the uvc_video_decode_isoc() function (drivers/media/usb/uvc/uvc_video.c). I checked on the xhci driver and the only place where this error code (-EXDEV) is assigned to frame->status is inside the skip_isoc_td() function (drivers/usb/host/xhci-ring.c). At this point I'm not sure if this is a bug on the xhci driver, another quirk needed by the XHCI_NEC_HOST, a camera misconfiguration on the USB Video Class driver or a firmware/hardware bug. The cameras are reported to work on the same machine but using another operating system (Windows). I was wondering if you can give me some pointers on how to be sure what's the issue or if this rings any bells to you. I've enabled both CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING in case you need more debugging information. Thanks a lot and best regards, Javier -- 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
