RE: [PATCH 1/4] usb: host: xhci-plat: add quirks member into struct xhci_plat_priv

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Tuesday, August 27, 2019 9:36 PM
> 
> Hi Shimoda-san,
> 
> On Tue, Aug 27, 2019 at 12:05 PM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> > To simplify adding xhci->quirks instead of the .init_quirk()
> > function, this patch adds a new parameter "quirks" into
> > the struct xhci_plat_priv.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

Thank you for your review!

> > --- a/drivers/usb/host/xhci-plat.h
> > +++ b/drivers/usb/host/xhci-plat.h
> > @@ -12,10 +12,12 @@
> >
> >  struct xhci_plat_priv {
> >         const char *firmware_name;
> > +       unsigned long long quirks;
> >         void (*plat_start)(struct usb_hcd *);
> >         int (*init_quirk)(struct usb_hcd *);
> >         int (*resume_quirk)(struct usb_hcd *);
> >  };
> >
> >  #define hcd_to_xhci_priv(h) ((struct xhci_plat_priv *)hcd_to_xhci(h)->priv)
> > +#define xhci_to_priv(x) ((struct xhci_plat_priv *)(x)->priv)
> 
> Just wondering: is x->priv guaranteed to be of type struct xhci_plat_priv *,
> also in the future?
> 
>     struct xhci_hcd {
>             ...
>             unsigned long           priv[0] __aligned(sizeof(s64));

It seems so. But, I'm not sure that we can change type of the priv[0] to
struct xhci_plat_priv *, because this implementation is related to all usb_hcd drivers:

< Details if you are interested in :) >
--- include/linux/usb/hcd.h ---
struct usb_hcd {
	...
	/* The HC driver's private data is stored at the end of
	 * this structure.
	 */
	unsigned long hcd_priv[0]
			__attribute__ ((aligned(sizeof(s64))));
};

--- drivers/usb/host/xhci.c ---
static const struct hc_driver xhci_hc_driver = {
	...
	.hcd_priv_size =	sizeof(struct xhci_hcd),

and

void xhci_init_driver(struct hc_driver *drv,
		      const struct xhci_driver_overrides *over)
{
	BUG_ON(!over);

	/* Copy the generic table to drv then apply the overrides */
	*drv = xhci_hc_driver;

	if (over) {
		drv->hcd_priv_size += over->extra_priv_size;

--- drivers/usb/host/xhci-plat.c ---
static const struct xhci_driver_overrides xhci_plat_overrides __initconst = {
	.extra_priv_size = sizeof(struct xhci_plat_priv),

void xhci_init_driver(struct hc_driver *drv,
		      const struct xhci_driver_overrides *over)
{
	BUG_ON(!over);

	/* Copy the generic table to drv then apply the overrides */
	*drv = xhci_hc_driver;

	if (over) {
		drv->hcd_priv_size += over->extra_priv_size;

--- drivers/usb/core/hcd.c ---
struct usb_hcd *__usb_create_hcd(const struct hc_driver *driver,
		struct device *sysdev, struct device *dev, const char *bus_name,
		struct usb_hcd *primary_hcd)
{
	struct usb_hcd *hcd;

	hcd = kzalloc(sizeof(*hcd) + driver->hcd_priv_size, GFP_KERNEL);
------------------------------

Best regards,
Yoshihiro Shimoda

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux