linux-kernel - Re: [PATCH v3 1/3] usb: typec: Separate USB Power Delivery from USB Type-C

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YmfgfRA1ecJwf12i@kroah.com>
Date:   Tue, 26 Apr 2022 14:07:25 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:     Benson Leung <bleung@...gle.com>,
        Prashant Malani <pmalani@...omium.org>,
        Jameson Thies <jthies@...gle.com>,
        "Regupathy, Rajaram" <rajaram.regupathy@...el.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Won Chung <wonchung@...gle.com>, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/3] usb: typec: Separate USB Power Delivery from USB
 Type-C

On Mon, Apr 25, 2022 at 03:49:44PM +0300, Heikki Krogerus wrote:
> --- /dev/null
> +++ b/drivers/usb/typec/pd.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __USB_POWER_DELIVERY__
> +#define __USB_POWER_DELIVERY__
> +
> +#include <linux/kobject.h>

Why kobject.h when:

> +
> +struct pd_capabilities {
> +	struct device dev;

This is a device?

> +	struct pd *pd;
> +	enum typec_role role;
> +};
> +
> +struct pd {
> +	struct device		dev;
> +	int			id;
> +
> +	u16			revision; /* 0300H = "3.0" */

So BCD?

> +	u16			version;
> +};

> +
> +#define to_pd_capabilities(o) container_of(o, struct pd_capabilities, dev)
> +#define to_pd(o) container_of(o, struct pd, dev)
> +
> +struct pd *pd_find(const char *name);

"struct pd" is just about the shortest structure name I've seen in the
kernel so far.  How about using some more letters?  :)

> +
> +int pd_init(void);
> +void pd_exit(void);

The kobject question above goes to the code as well.  You are creating a
bunch of raw kobjects still, why?  This should all fit into the driver
model and kobjects shouldn't be needed.  Are you trying to nest too deep
in the attributes?  If so, kobjects will not work as userspace tools
will not realize they are there and are attributes at all.

Try running some tests using libudev and see the lack of attributes with
this patch, tools are not going to be able to see them at all this way.

thanks,

greg k-h