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
