| Message ID | 1444217938-4668-2-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On Wed, Oct 07, 2015 at 08:38:53PM +0900, Yoshihiro Shimoda wrote: > This patch adds an xhci->priv field for storing the of_device_id.data > pointer. This will simplify the code to match platform specific > variables (e.g. platform type and firmware name). > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > drivers/usb/host/xhci.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > index 51093df..da227ba 100644 > --- a/drivers/usb/host/xhci.h > +++ b/drivers/usb/host/xhci.h > @@ -1653,6 +1653,9 @@ struct xhci_hcd { > u32 port_status_u0; > /* Compliance Mode Timer Triggered every 2 seconds */ > #define COMP_MODE_RCVRY_MSECS 2000 > + > + /* Store of_device_id.data pointer as xhci-plat private data */ > + const void *priv; > }; Why can't we do what we do with the other *_hcd structures and pad out the end of the structure as needed with the following: /* platform-specific data -- must come last */ unsigned long priv[0] __aligned(sizeof(s64)); and then ask for the requested amount of space depending on the platform being used? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Greg, > Sent: Saturday, October 17, 2015 3:38 PM > > On Wed, Oct 07, 2015 at 08:38:53PM +0900, Yoshihiro Shimoda wrote: > > This patch adds an xhci->priv field for storing the of_device_id.data > > pointer. This will simplify the code to match platform specific > > variables (e.g. platform type and firmware name). > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > --- > > drivers/usb/host/xhci.h | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > > index 51093df..da227ba 100644 > > --- a/drivers/usb/host/xhci.h > > +++ b/drivers/usb/host/xhci.h > > @@ -1653,6 +1653,9 @@ struct xhci_hcd { > > u32 port_status_u0; > > /* Compliance Mode Timer Triggered every 2 seconds */ > > #define COMP_MODE_RCVRY_MSECS 2000 > > + > > + /* Store of_device_id.data pointer as xhci-plat private data */ > > + const void *priv; > > }; > > Why can't we do what we do with the other *_hcd structures and pad out > the end of the structure as needed with the following: > /* platform-specific data -- must come last */ > unsigned long priv[0] __aligned(sizeof(s64)); > > and then ask for the requested amount of space depending on the platform > being used? Since I found the following comment from Thomas before I submitted, I didn't add such the other *_hcd structures to avoid complicate. http://thread.gmane.org/gmane.linux.kernel/1697250/focus=73028 (This comment was written in last year though...) The current code has "extra_priv_size" like the following patch. So, can we add "priv" to xhci_hcd like the other *_hcd structures? https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=cd33a32157e42483ffea31e32b1cee4f11ff9592 Anyway, I will investigate this "extra_priv_size". Best regards, Yoshihiro Shimoda > thanks, > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Greg, > From: Yoshihiro Shimoda > Sent: Monday, October 19, 2015 6:11 PM > > Hi Greg, > > > Sent: Saturday, October 17, 2015 3:38 PM > > > > On Wed, Oct 07, 2015 at 08:38:53PM +0900, Yoshihiro Shimoda wrote: > > > This patch adds an xhci->priv field for storing the of_device_id.data > > > pointer. This will simplify the code to match platform specific > > > variables (e.g. platform type and firmware name). > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > --- > > > drivers/usb/host/xhci.h | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > > > index 51093df..da227ba 100644 > > > --- a/drivers/usb/host/xhci.h > > > +++ b/drivers/usb/host/xhci.h > > > @@ -1653,6 +1653,9 @@ struct xhci_hcd { > > > u32 port_status_u0; > > > /* Compliance Mode Timer Triggered every 2 seconds */ > > > #define COMP_MODE_RCVRY_MSECS 2000 > > > + > > > + /* Store of_device_id.data pointer as xhci-plat private data */ > > > + const void *priv; > > > }; > > > > Why can't we do what we do with the other *_hcd structures and pad out > > the end of the structure as needed with the following: > > /* platform-specific data -- must come last */ > > unsigned long priv[0] __aligned(sizeof(s64)); > > > > and then ask for the requested amount of space depending on the platform > > being used? > > Since I found the following comment from Thomas before I submitted, > I didn't add such the other *_hcd structures to avoid complicate. > http://thread.gmane.org/gmane.linux.kernel/1697250/focus=73028 > (This comment was written in last year though...) > > The current code has "extra_priv_size" like the following patch. > So, can we add "priv" to xhci_hcd like the other *_hcd structures? > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=cd33a32157e42483ffea31e32b1cee4f11ff9592 > > Anyway, I will investigate this "extra_priv_size". I investgated this topic. I found that the current code can add such the other *_hcd structures using the "extra_priv_size". (In other words, the comment from Thomas in last year is not fit with the current xhci driver code.) However, the current implementation is strange to me: - The hcd_priv_size in xhci.c is sizeof(struct xhci_hcd *). - The extra_priv_size in xhci-plat.c and xhci-pci are sizeof(struct xhci_hcd). In this means, actual hcd_priv_size in the xhci driver is: sizeof(struct xhci_hcd) + sizeof(struct xhci_hcd *) I don't think that the "+ sizeof(struct xhci_hcd *)" is needed. Is my understanding correct? If so, I will fix this and add such a patch into my v3 patch set. Best regards, Yoshihiro Shimoda > Best regards, > Yoshihiro Shimoda > > > thanks, > > > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 51093df..da227ba 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1653,6 +1653,9 @@ struct xhci_hcd { u32 port_status_u0; /* Compliance Mode Timer Triggered every 2 seconds */ #define COMP_MODE_RCVRY_MSECS 2000 + + /* Store of_device_id.data pointer as xhci-plat private data */ + const void *priv; }; /* Platform specific overrides to generic XHCI hc_driver ops */
This patch adds an xhci->priv field for storing the of_device_id.data pointer. This will simplify the code to match platform specific variables (e.g. platform type and firmware name). Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/usb/host/xhci.h | 3 +++ 1 file changed, 3 insertions(+)