| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <qhfmvqjxad3ngk3azdkyxhw6ka7ijxccl6d2ylu26zlmovmeg3@3f2r4f6ksr6s>
Date: Sun, 24 Aug 2025 20:55:09 +0200
From: Richard Leitner <richard.leitner@...ux.dev>
To: Jisheng Zhang <jszhang@...nel.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, linux-usb@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/3] usb: usb251xb: support usage case without I2C
control
Hi Jisheng,
On Thu, Aug 21, 2025 at 12:17:43AM +0800, Jisheng Zhang wrote:
> Currently, the usb251xb assumes i2c control. But from HW point of
> view, the hub supports usage case without any i2c, we only want the
> gpio controls.
>
> Refactor the code so that register writes for configuration are only
> performed if the device has a i2c_client provided and also register as
> a platform driver. This allows the driver to be used to manage GPIO
> based control of the device.
>
> Signed-off-by: Jisheng Zhang <jszhang@...nel.org>
> ---
> drivers/usb/misc/usb251xb.c | 108 +++++++++++++++++++++++++++++++-----
> 1 file changed, 94 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
> index cb2f946de42c..e9a9404d17b2 100644
> --- a/drivers/usb/misc/usb251xb.c
> +++ b/drivers/usb/misc/usb251xb.c
> @@ -17,6 +17,7 @@
> #include <linux/module.h>
> #include <linux/nls.h>
> #include <linux/of.h>
> +#include <linux/platform_device.h>
> #include <linux/regulator/consumer.h>
> #include <linux/slab.h>
>
> @@ -242,15 +243,19 @@ static int usb251xb_check_dev_children(struct device *dev, void *child)
> static int usb251x_check_gpio_chip(struct usb251xb *hub)
> {
> struct gpio_chip *gc = gpiod_to_chip(hub->gpio_reset);
> - struct i2c_adapter *adap = hub->i2c->adapter;
> + struct i2c_adapter *adap;
> int ret;
>
> + if (!hub->i2c)
> + return 0;
> +
> if (!hub->gpio_reset)
> return 0;
>
> if (!gc)
> return -EINVAL;
>
> + adap = hub->i2c->adapter;
> ret = usb251xb_check_dev_children(&adap->dev, gc->parent);
> if (ret) {
> dev_err(hub->dev, "Reset GPIO chip is at the same i2c-bus\n");
> @@ -271,7 +276,8 @@ static void usb251xb_reset(struct usb251xb *hub)
> if (!hub->gpio_reset)
> return;
>
> - i2c_lock_bus(hub->i2c->adapter, I2C_LOCK_SEGMENT);
> + if (hub->i2c)
> + i2c_lock_bus(hub->i2c->adapter, I2C_LOCK_SEGMENT);
>
> gpiod_set_value_cansleep(hub->gpio_reset, 1);
> usleep_range(1, 10); /* >=1us RESET_N asserted */
> @@ -280,7 +286,8 @@ static void usb251xb_reset(struct usb251xb *hub)
> /* wait for hub recovery/stabilization */
> usleep_range(500, 750); /* >=500us after RESET_N deasserted */
>
> - i2c_unlock_bus(hub->i2c->adapter, I2C_LOCK_SEGMENT);
> + if (hub->i2c)
> + i2c_unlock_bus(hub->i2c->adapter, I2C_LOCK_SEGMENT);
> }
>
> static int usb251xb_connect(struct usb251xb *hub)
> @@ -289,7 +296,11 @@ static int usb251xb_connect(struct usb251xb *hub)
> int err, i;
> char i2c_wb[USB251XB_I2C_REG_SZ];
>
> - memset(i2c_wb, 0, USB251XB_I2C_REG_SZ);
> + if (!hub->i2c) {
> + usb251xb_reset(hub);
> + dev_info(dev, "hub is put in default configuration.\n");
> + return 0;
> + }
You dropped the memset of write buffer. Are you sure this doesn't cause
any issues? (Sorry, I'm currently travelling, so I wasn't able to test
the series on real hw)
Apart from that the patch LGTM. Thanks! :-)
>
> if (hub->skip_config) {
> dev_info(dev, "Skip hub configuration, only attach.\n");
> @@ -698,18 +709,13 @@ static int usb251xb_i2c_probe(struct i2c_client *i2c)
> return usb251xb_probe(hub);
> }
>
> -static int usb251xb_suspend(struct device *dev)
> +static int usb251xb_suspend(struct usb251xb *hub)
> {
> - struct i2c_client *client = to_i2c_client(dev);
> - struct usb251xb *hub = i2c_get_clientdata(client);
> -
> return regulator_disable(hub->vdd);
> }
>
> -static int usb251xb_resume(struct device *dev)
> +static int usb251xb_resume(struct usb251xb *hub)
> {
> - struct i2c_client *client = to_i2c_client(dev);
> - struct usb251xb *hub = i2c_get_clientdata(client);
> int err;
>
> err = regulator_enable(hub->vdd);
> @@ -719,7 +725,23 @@ static int usb251xb_resume(struct device *dev)
> return usb251xb_connect(hub);
> }
>
> -static DEFINE_SIMPLE_DEV_PM_OPS(usb251xb_pm_ops, usb251xb_suspend, usb251xb_resume);
> +static int usb251xb_i2c_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct usb251xb *hub = i2c_get_clientdata(client);
> +
> + return usb251xb_suspend(hub);
> +}
> +
> +static int usb251xb_i2c_resume(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct usb251xb *hub = i2c_get_clientdata(client);
> +
> + return usb251xb_resume(hub);
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(usb251xb_i2c_pm_ops, usb251xb_i2c_suspend, usb251xb_i2c_resume);
>
> static const struct i2c_device_id usb251xb_id[] = {
> { "usb2422" },
> @@ -739,13 +761,71 @@ static struct i2c_driver usb251xb_i2c_driver = {
> .driver = {
> .name = DRIVER_NAME,
> .of_match_table = usb251xb_of_match,
> - .pm = pm_sleep_ptr(&usb251xb_pm_ops),
> + .pm = pm_sleep_ptr(&usb251xb_i2c_pm_ops),
> },
> .probe = usb251xb_i2c_probe,
> .id_table = usb251xb_id,
> };
>
> -module_i2c_driver(usb251xb_i2c_driver);
> +static int usb251xb_plat_probe(struct platform_device *pdev)
> +{
> + struct usb251xb *hub;
> +
> + hub = devm_kzalloc(&pdev->dev, sizeof(*hub), GFP_KERNEL);
> + if (!hub)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, hub);
> + hub->dev = &pdev->dev;
> +
> + return usb251xb_probe(hub);
> +}
> +
> +static int usb251xb_plat_suspend(struct device *dev)
> +{
> + return usb251xb_suspend(dev_get_drvdata(dev));
> +}
> +
> +static int usb251xb_plat_resume(struct device *dev)
> +{
> + return usb251xb_resume(dev_get_drvdata(dev));
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(usb251xb_plat_pm_ops, usb251xb_plat_suspend, usb251xb_plat_resume);
> +
> +static struct platform_driver usb251xb_plat_driver = {
> + .driver = {
> + .name = DRIVER_NAME,
> + .of_match_table = usb251xb_of_match,
> + .pm = pm_sleep_ptr(&usb251xb_plat_pm_ops),
> + },
> + .probe = usb251xb_plat_probe,
> +};
> +
> +static int __init usb251xb_init(void)
> +{
> + int err;
> +
> + err = i2c_add_driver(&usb251xb_i2c_driver);
> + if (err)
> + return err;
> +
> + err = platform_driver_register(&usb251xb_plat_driver);
> + if (err) {
> + i2c_del_driver(&usb251xb_i2c_driver);
> + return err;
> + }
> +
> + return 0;
> +}
> +module_init(usb251xb_init);
> +
> +static void __exit usb251xb_exit(void)
> +{
> + platform_driver_unregister(&usb251xb_plat_driver);
> + i2c_del_driver(&usb251xb_i2c_driver);
> +}
> +module_exit(usb251xb_exit);
>
> MODULE_AUTHOR("Richard Leitner <richard.leitner@...data.com>");
> MODULE_DESCRIPTION("USB251x/xBi USB 2.0 Hub Controller Driver");
> --
> 2.50.0
>
regards;rl
Powered by blists - more mailing lists