linux-kernel - Re: [RFC PATCH 1/3] usb: Add USB repeater generic framework

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]

Date:   Fri, 18 Nov 2022 09:59:43 +0100
From:   Andrzej Pietrasiewicz <andrzej.p@...labora.com>
To:     Abel Vesa <abel.vesa@...aro.org>, Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-scsi@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [RFC PATCH 1/3] usb: Add USB repeater generic framework

Hi Abel,

W dniu 16.11.2022 o 13:30, Abel Vesa pisze:
> With more SoCs moving towards eUSB2, 

Can you name a few?

such platforms will have to use
> a USB 2.0 compliance repeater. This repeater HW-wise usually deals with
> level shifting, but depending on the implementation it can do much more.
> So add a ganeric USB framework that would allow either a generic PHY or
> some USB host controller to control and get a repeater from a devicetree
> phandle. This framework will further be used by platform specific
> drivers to register the repeater and implement platform specific ops.
> 
> Signed-off-by: Abel Vesa <abel.vesa@...aro.org>
> ---
>   drivers/usb/Kconfig             |   2 +
>   drivers/usb/Makefile            |   2 +
>   drivers/usb/repeater/Kconfig    |   9 ++
>   drivers/usb/repeater/Makefile   |   6 +
>   drivers/usb/repeater/repeater.c | 198 ++++++++++++++++++++++++++++++++
>   include/linux/usb/repeater.h    |  78 +++++++++++++
>   6 files changed, 295 insertions(+)
>   create mode 100644 drivers/usb/repeater/Kconfig
>   create mode 100644 drivers/usb/repeater/Makefile
>   create mode 100644 drivers/usb/repeater/repeater.c
>   create mode 100644 include/linux/usb/repeater.h
> 

<snip>

> diff --git a/include/linux/usb/repeater.h b/include/linux/usb/repeater.h
> new file mode 100644
> index 000000000000..e68e0936f1e5
> --- /dev/null
> +++ b/include/linux/usb/repeater.h
> @@ -0,0 +1,78 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * USB Repeater defines
> + */
> +
> +#ifndef __LINUX_USB_REPEATER_H
> +#define __LINUX_USB_REPEATER_H
> +
> +struct usb_repeater {
> +	struct device		*dev;
> +	const char		*label;
> +	unsigned int		 flags;
> +
> +	struct list_head	head;

This member serves the purpose of a list _entry_, no?
The _head_ is static LIST_HEAD(usb_repeater_list);
Maybe call it "entry"?

> +	int	(*reset)(struct usb_repeater *rptr, bool assert);
> +	int	(*init)(struct usb_repeater *rptr);
> +	int	(*power_on)(struct usb_repeater *rptr);
> +	int	(*power_off)(struct usb_repeater *rptr);

Would you document these ops somehow? Potential driver writers need to
understand when they are called and what they are supposed to do.
In particular, how do these relate to what's in "Embedded USB2 (eUSB2)
Physical Layer Supplement to the USB Revision 2.0 Specification"?

Regards,

Andrzej