| Message ID | alpine.LFD.2.11.1404171938190.1588@denkbrett (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Thu, 2014-04-17 at 19:46 +0200, Sebastian Ott wrote: > pci: move open fabric devspec attribute to pci common code > > Move the devspec OF attribute to pci common code's set of device > attributes since it's not architecture dependent. > As a side effect microblaze and powerpc no longer need to use > pcibios_add_platform_entries. > > Link: https://lkml.kernel.org/r/alpine.LFD.2.11.1404141101500.1529@denkbrett > Signed-off-by: Sebastian Ott <sebott@linux.vnet.ibm.com> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> (Pending you've compile tested it on powerpc, I'm on vacation and haven't done it :) > --- > arch/microblaze/pci/pci-common.c | 20 -------------------- > arch/powerpc/kernel/pci-common.c | 20 -------------------- > drivers/pci/pci-sysfs.c | 17 +++++++++++++++++ > 3 files changed, 17 insertions(+), 40 deletions(-) > > --- a/arch/microblaze/pci/pci-common.c > +++ b/arch/microblaze/pci/pci-common.c > @@ -168,26 +168,6 @@ struct pci_controller *pci_find_hose_for > return NULL; > } > > -static ssize_t pci_show_devspec(struct device *dev, > - struct device_attribute *attr, char *buf) > -{ > - struct pci_dev *pdev; > - struct device_node *np; > - > - pdev = to_pci_dev(dev); > - np = pci_device_to_OF_node(pdev); > - if (np == NULL || np->full_name == NULL) > - return 0; > - return sprintf(buf, "%s", np->full_name); > -} > -static DEVICE_ATTR(devspec, S_IRUGO, pci_show_devspec, NULL); > - > -/* Add sysfs properties */ > -int pcibios_add_platform_entries(struct pci_dev *pdev) > -{ > - return device_create_file(&pdev->dev, &dev_attr_devspec); > -} > - > void pcibios_set_master(struct pci_dev *dev) > { > /* No special bus mastering setup handling */ > --- a/arch/powerpc/kernel/pci-common.c > +++ b/arch/powerpc/kernel/pci-common.c > @@ -201,26 +201,6 @@ struct pci_controller* pci_find_hose_for > return NULL; > } > > -static ssize_t pci_show_devspec(struct device *dev, > - struct device_attribute *attr, char *buf) > -{ > - struct pci_dev *pdev; > - struct device_node *np; > - > - pdev = to_pci_dev (dev); > - np = pci_device_to_OF_node(pdev); > - if (np == NULL || np->full_name == NULL) > - return 0; > - return sprintf(buf, "%s", np->full_name); > -} > -static DEVICE_ATTR(devspec, S_IRUGO, pci_show_devspec, NULL); > - > -/* Add sysfs properties */ > -int pcibios_add_platform_entries(struct pci_dev *pdev) > -{ > - return device_create_file(&pdev->dev, &dev_attr_devspec); > -} > - > /* > * Reads the interrupt pin to determine if interrupt is use by card. > * If the interrupt is used, then gets the interrupt line from the > --- a/drivers/pci/pci-sysfs.c > +++ b/drivers/pci/pci-sysfs.c > @@ -416,6 +416,20 @@ static ssize_t d3cold_allowed_show(struc > static DEVICE_ATTR_RW(d3cold_allowed); > #endif > > +#ifdef CONFIG_OF > +static ssize_t devspec_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct pci_dev *pdev = to_pci_dev(dev); > + struct device_node *np = pci_device_to_OF_node(pdev); > + > + if (np == NULL || np->full_name == NULL) > + return 0; > + return sprintf(buf, "%s", np->full_name); > +} > +static DEVICE_ATTR_RO(devspec); > +#endif > + > #ifdef CONFIG_PCI_IOV > static ssize_t sriov_totalvfs_show(struct device *dev, > struct device_attribute *attr, > @@ -521,6 +535,9 @@ static struct attribute *pci_dev_attrs[] > #if defined(CONFIG_PM_RUNTIME) && defined(CONFIG_ACPI) > &dev_attr_d3cold_allowed.attr, > #endif > +#ifdef CONFIG_OF > + &dev_attr_devspec.attr, > +#endif > NULL, > }; > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 18 Apr 2014, Benjamin Herrenschmidt wrote: > On Thu, 2014-04-17 at 19:46 +0200, Sebastian Ott wrote: > > > pci: move open fabric devspec attribute to pci common code > > > > Move the devspec OF attribute to pci common code's set of device > > attributes since it's not architecture dependent. > > As a side effect microblaze and powerpc no longer need to use > > pcibios_add_platform_entries. > > > > Link: https://lkml.kernel.org/r/alpine.LFD.2.11.1404141101500.1529@denkbrett > > Signed-off-by: Sebastian Ott <sebott@linux.vnet.ibm.com> > > Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > (Pending you've compile tested it on powerpc, I'm on vacation and > haven't done it :) I did: make ARCH=powerpc CROSS_COMPILE=powerpc64-4.3.2- g5_defconfig make ARCH=powerpc CROSS_COMPILE=powerpc64-4.3.2- and failed with: CC arch/powerpc/lib/xor_vmx.o In file included from include/linux/list.h:4, from include/linux/preempt.h:10, from arch/powerpc/lib/xor_vmx.c:22: include/linux/types.h:29: error: both ‘unsigned’ and ‘_Bool’ in declaration specifiers cc1: warnings being treated as errors include/linux/types.h:29: error: useless type name in empty declaration make ARCH=powerpc CROSS_COMPILE=powerpc64-4.3.2- arch/powerpc/kernel/ drivers/pci/ worked just fine. Regards, Sebastian > > > --- > > arch/microblaze/pci/pci-common.c | 20 -------------------- > > arch/powerpc/kernel/pci-common.c | 20 -------------------- > > drivers/pci/pci-sysfs.c | 17 +++++++++++++++++ > > 3 files changed, 17 insertions(+), 40 deletions(-) > > > > --- a/arch/microblaze/pci/pci-common.c > > +++ b/arch/microblaze/pci/pci-common.c > > @@ -168,26 +168,6 @@ struct pci_controller *pci_find_hose_for > > return NULL; > > } > > > > -static ssize_t pci_show_devspec(struct device *dev, > > - struct device_attribute *attr, char *buf) > > -{ > > - struct pci_dev *pdev; > > - struct device_node *np; > > - > > - pdev = to_pci_dev(dev); > > - np = pci_device_to_OF_node(pdev); > > - if (np == NULL || np->full_name == NULL) > > - return 0; > > - return sprintf(buf, "%s", np->full_name); > > -} > > -static DEVICE_ATTR(devspec, S_IRUGO, pci_show_devspec, NULL); > > - > > -/* Add sysfs properties */ > > -int pcibios_add_platform_entries(struct pci_dev *pdev) > > -{ > > - return device_create_file(&pdev->dev, &dev_attr_devspec); > > -} > > - > > void pcibios_set_master(struct pci_dev *dev) > > { > > /* No special bus mastering setup handling */ > > --- a/arch/powerpc/kernel/pci-common.c > > +++ b/arch/powerpc/kernel/pci-common.c > > @@ -201,26 +201,6 @@ struct pci_controller* pci_find_hose_for > > return NULL; > > } > > > > -static ssize_t pci_show_devspec(struct device *dev, > > - struct device_attribute *attr, char *buf) > > -{ > > - struct pci_dev *pdev; > > - struct device_node *np; > > - > > - pdev = to_pci_dev (dev); > > - np = pci_device_to_OF_node(pdev); > > - if (np == NULL || np->full_name == NULL) > > - return 0; > > - return sprintf(buf, "%s", np->full_name); > > -} > > -static DEVICE_ATTR(devspec, S_IRUGO, pci_show_devspec, NULL); > > - > > -/* Add sysfs properties */ > > -int pcibios_add_platform_entries(struct pci_dev *pdev) > > -{ > > - return device_create_file(&pdev->dev, &dev_attr_devspec); > > -} > > - > > /* > > * Reads the interrupt pin to determine if interrupt is use by card. > > * If the interrupt is used, then gets the interrupt line from the > > --- a/drivers/pci/pci-sysfs.c > > +++ b/drivers/pci/pci-sysfs.c > > @@ -416,6 +416,20 @@ static ssize_t d3cold_allowed_show(struc > > static DEVICE_ATTR_RW(d3cold_allowed); > > #endif > > > > +#ifdef CONFIG_OF > > +static ssize_t devspec_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + struct pci_dev *pdev = to_pci_dev(dev); > > + struct device_node *np = pci_device_to_OF_node(pdev); > > + > > + if (np == NULL || np->full_name == NULL) > > + return 0; > > + return sprintf(buf, "%s", np->full_name); > > +} > > +static DEVICE_ATTR_RO(devspec); > > +#endif > > + > > #ifdef CONFIG_PCI_IOV > > static ssize_t sriov_totalvfs_show(struct device *dev, > > struct device_attribute *attr, > > @@ -521,6 +535,9 @@ static struct attribute *pci_dev_attrs[] > > #if defined(CONFIG_PM_RUNTIME) && defined(CONFIG_ACPI) > > &dev_attr_d3cold_allowed.attr, > > #endif > > +#ifdef CONFIG_OF > > + &dev_attr_devspec.attr, > > +#endif > > NULL, > > }; > > > > >
On Fri, 2014-04-18 at 12:10 +0200, Sebastian Ott wrote: > On Fri, 18 Apr 2014, Benjamin Herrenschmidt wrote: > > On Thu, 2014-04-17 at 19:46 +0200, Sebastian Ott wrote: > > > > > pci: move open fabric devspec attribute to pci common code > > > > > > Move the devspec OF attribute to pci common code's set of device > > > attributes since it's not architecture dependent. > > > As a side effect microblaze and powerpc no longer need to use > > > pcibios_add_platform_entries. > > > > > > Link: https://lkml.kernel.org/r/alpine.LFD.2.11.1404141101500.1529@denkbrett > > > Signed-off-by: Sebastian Ott <sebott@linux.vnet.ibm.com> > > > > Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > > > (Pending you've compile tested it on powerpc, I'm on vacation and > > haven't done it :) > > I did: > make ARCH=powerpc CROSS_COMPILE=powerpc64-4.3.2- g5_defconfig > make ARCH=powerpc CROSS_COMPILE=powerpc64-4.3.2- > > and failed with: > CC arch/powerpc/lib/xor_vmx.o > In file included from include/linux/list.h:4, > from include/linux/preempt.h:10, > from arch/powerpc/lib/xor_vmx.c:22: > include/linux/types.h:29: error: both ‘unsigned’ and ‘_Bool’ in declaration specifiers > cc1: warnings being treated as errors > include/linux/types.h:29: error: useless type name in empty declaration Not sure what's up here, did you try with a more recent compiler ? 4.3 is pretty ancient by kernel standards. You can find cross compilers there: https://www.kernel.org/pub/tools/crosstool/ They aren't the newest either but they should work. Ben. > > make ARCH=powerpc CROSS_COMPILE=powerpc64-4.3.2- arch/powerpc/kernel/ drivers/pci/ > > worked just fine. > > Regards, > Sebastian > > > > > > --- > > > arch/microblaze/pci/pci-common.c | 20 -------------------- > > > arch/powerpc/kernel/pci-common.c | 20 -------------------- > > > drivers/pci/pci-sysfs.c | 17 +++++++++++++++++ > > > 3 files changed, 17 insertions(+), 40 deletions(-) > > > > > > --- a/arch/microblaze/pci/pci-common.c > > > +++ b/arch/microblaze/pci/pci-common.c > > > @@ -168,26 +168,6 @@ struct pci_controller *pci_find_hose_for > > > return NULL; > > > } > > > > > > -static ssize_t pci_show_devspec(struct device *dev, > > > - struct device_attribute *attr, char *buf) > > > -{ > > > - struct pci_dev *pdev; > > > - struct device_node *np; > > > - > > > - pdev = to_pci_dev(dev); > > > - np = pci_device_to_OF_node(pdev); > > > - if (np == NULL || np->full_name == NULL) > > > - return 0; > > > - return sprintf(buf, "%s", np->full_name); > > > -} > > > -static DEVICE_ATTR(devspec, S_IRUGO, pci_show_devspec, NULL); > > > - > > > -/* Add sysfs properties */ > > > -int pcibios_add_platform_entries(struct pci_dev *pdev) > > > -{ > > > - return device_create_file(&pdev->dev, &dev_attr_devspec); > > > -} > > > - > > > void pcibios_set_master(struct pci_dev *dev) > > > { > > > /* No special bus mastering setup handling */ > > > --- a/arch/powerpc/kernel/pci-common.c > > > +++ b/arch/powerpc/kernel/pci-common.c > > > @@ -201,26 +201,6 @@ struct pci_controller* pci_find_hose_for > > > return NULL; > > > } > > > > > > -static ssize_t pci_show_devspec(struct device *dev, > > > - struct device_attribute *attr, char *buf) > > > -{ > > > - struct pci_dev *pdev; > > > - struct device_node *np; > > > - > > > - pdev = to_pci_dev (dev); > > > - np = pci_device_to_OF_node(pdev); > > > - if (np == NULL || np->full_name == NULL) > > > - return 0; > > > - return sprintf(buf, "%s", np->full_name); > > > -} > > > -static DEVICE_ATTR(devspec, S_IRUGO, pci_show_devspec, NULL); > > > - > > > -/* Add sysfs properties */ > > > -int pcibios_add_platform_entries(struct pci_dev *pdev) > > > -{ > > > - return device_create_file(&pdev->dev, &dev_attr_devspec); > > > -} > > > - > > > /* > > > * Reads the interrupt pin to determine if interrupt is use by card. > > > * If the interrupt is used, then gets the interrupt line from the > > > --- a/drivers/pci/pci-sysfs.c > > > +++ b/drivers/pci/pci-sysfs.c > > > @@ -416,6 +416,20 @@ static ssize_t d3cold_allowed_show(struc > > > static DEVICE_ATTR_RW(d3cold_allowed); > > > #endif > > > > > > +#ifdef CONFIG_OF > > > +static ssize_t devspec_show(struct device *dev, > > > + struct device_attribute *attr, char *buf) > > > +{ > > > + struct pci_dev *pdev = to_pci_dev(dev); > > > + struct device_node *np = pci_device_to_OF_node(pdev); > > > + > > > + if (np == NULL || np->full_name == NULL) > > > + return 0; > > > + return sprintf(buf, "%s", np->full_name); > > > +} > > > +static DEVICE_ATTR_RO(devspec); > > > +#endif > > > + > > > #ifdef CONFIG_PCI_IOV > > > static ssize_t sriov_totalvfs_show(struct device *dev, > > > struct device_attribute *attr, > > > @@ -521,6 +535,9 @@ static struct attribute *pci_dev_attrs[] > > > #if defined(CONFIG_PM_RUNTIME) && defined(CONFIG_ACPI) > > > &dev_attr_d3cold_allowed.attr, > > > #endif > > > +#ifdef CONFIG_OF > > > + &dev_attr_devspec.attr, > > > +#endif > > > NULL, > > > }; > > > > > > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 19 Apr 2014, Benjamin Herrenschmidt wrote: > On Fri, 2014-04-18 at 12:10 +0200, Sebastian Ott wrote: > > On Fri, 18 Apr 2014, Benjamin Herrenschmidt wrote: > > > On Thu, 2014-04-17 at 19:46 +0200, Sebastian Ott wrote: > > > > > > > pci: move open fabric devspec attribute to pci common code > > > > > > > > Move the devspec OF attribute to pci common code's set of device > > > > attributes since it's not architecture dependent. > > > > As a side effect microblaze and powerpc no longer need to use > > > > pcibios_add_platform_entries. > > > > > > > > Link: https://lkml.kernel.org/r/alpine.LFD.2.11.1404141101500.1529@denkbrett > > > > Signed-off-by: Sebastian Ott <sebott@linux.vnet.ibm.com> > > > > > > Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > > > > > (Pending you've compile tested it on powerpc, I'm on vacation and > > > haven't done it :) > > > > I did: > > make ARCH=powerpc CROSS_COMPILE=powerpc64-4.3.2- g5_defconfig > > make ARCH=powerpc CROSS_COMPILE=powerpc64-4.3.2- > > > > and failed with: > > CC arch/powerpc/lib/xor_vmx.o > > In file included from include/linux/list.h:4, > > from include/linux/preempt.h:10, > > from arch/powerpc/lib/xor_vmx.c:22: > > include/linux/types.h:29: error: both ‘unsigned’ and ‘_Bool’ in declaration specifiers > > cc1: warnings being treated as errors > > include/linux/types.h:29: error: useless type name in empty declaration > > Not sure what's up here, did you try with a more recent compiler ? 4.3 > is pretty ancient by kernel standards. > > You can find cross compilers there: > > https://www.kernel.org/pub/tools/crosstool/ > > They aren't the newest either but they should work. Yes. That did the trick. Thanks, Sebastian > > > > make ARCH=powerpc CROSS_COMPILE=powerpc64-4.3.2- arch/powerpc/kernel/ drivers/pci/ > > > > worked just fine. > > > > Regards, > > Sebastian > > > > > > > > > --- > > > > arch/microblaze/pci/pci-common.c | 20 -------------------- > > > > arch/powerpc/kernel/pci-common.c | 20 -------------------- > > > > drivers/pci/pci-sysfs.c | 17 +++++++++++++++++ > > > > 3 files changed, 17 insertions(+), 40 deletions(-) > > > > > > > > --- a/arch/microblaze/pci/pci-common.c > > > > +++ b/arch/microblaze/pci/pci-common.c > > > > @@ -168,26 +168,6 @@ struct pci_controller *pci_find_hose_for > > > > return NULL; > > > > } > > > > > > > > -static ssize_t pci_show_devspec(struct device *dev, > > > > - struct device_attribute *attr, char *buf) > > > > -{ > > > > - struct pci_dev *pdev; > > > > - struct device_node *np; > > > > - > > > > - pdev = to_pci_dev(dev); > > > > - np = pci_device_to_OF_node(pdev); > > > > - if (np == NULL || np->full_name == NULL) > > > > - return 0; > > > > - return sprintf(buf, "%s", np->full_name); > > > > -} > > > > -static DEVICE_ATTR(devspec, S_IRUGO, pci_show_devspec, NULL); > > > > - > > > > -/* Add sysfs properties */ > > > > -int pcibios_add_platform_entries(struct pci_dev *pdev) > > > > -{ > > > > - return device_create_file(&pdev->dev, &dev_attr_devspec); > > > > -} > > > > - > > > > void pcibios_set_master(struct pci_dev *dev) > > > > { > > > > /* No special bus mastering setup handling */ > > > > --- a/arch/powerpc/kernel/pci-common.c > > > > +++ b/arch/powerpc/kernel/pci-common.c > > > > @@ -201,26 +201,6 @@ struct pci_controller* pci_find_hose_for > > > > return NULL; > > > > } > > > > > > > > -static ssize_t pci_show_devspec(struct device *dev, > > > > - struct device_attribute *attr, char *buf) > > > > -{ > > > > - struct pci_dev *pdev; > > > > - struct device_node *np; > > > > - > > > > - pdev = to_pci_dev (dev); > > > > - np = pci_device_to_OF_node(pdev); > > > > - if (np == NULL || np->full_name == NULL) > > > > - return 0; > > > > - return sprintf(buf, "%s", np->full_name); > > > > -} > > > > -static DEVICE_ATTR(devspec, S_IRUGO, pci_show_devspec, NULL); > > > > - > > > > -/* Add sysfs properties */ > > > > -int pcibios_add_platform_entries(struct pci_dev *pdev) > > > > -{ > > > > - return device_create_file(&pdev->dev, &dev_attr_devspec); > > > > -} > > > > - > > > > /* > > > > * Reads the interrupt pin to determine if interrupt is use by card. > > > > * If the interrupt is used, then gets the interrupt line from the > > > > --- a/drivers/pci/pci-sysfs.c > > > > +++ b/drivers/pci/pci-sysfs.c > > > > @@ -416,6 +416,20 @@ static ssize_t d3cold_allowed_show(struc > > > > static DEVICE_ATTR_RW(d3cold_allowed); > > > > #endif > > > > > > > > +#ifdef CONFIG_OF > > > > +static ssize_t devspec_show(struct device *dev, > > > > + struct device_attribute *attr, char *buf) > > > > +{ > > > > + struct pci_dev *pdev = to_pci_dev(dev); > > > > + struct device_node *np = pci_device_to_OF_node(pdev); > > > > + > > > > + if (np == NULL || np->full_name == NULL) > > > > + return 0; > > > > + return sprintf(buf, "%s", np->full_name); > > > > +} > > > > +static DEVICE_ATTR_RO(devspec); > > > > +#endif > > > > + > > > > #ifdef CONFIG_PCI_IOV > > > > static ssize_t sriov_totalvfs_show(struct device *dev, > > > > struct device_attribute *attr, > > > > @@ -521,6 +535,9 @@ static struct attribute *pci_dev_attrs[] > > > > #if defined(CONFIG_PM_RUNTIME) && defined(CONFIG_ACPI) > > > > &dev_attr_d3cold_allowed.attr, > > > > #endif > > > > +#ifdef CONFIG_OF > > > > + &dev_attr_devspec.attr, > > > > +#endif > > > > NULL, > > > > }; > > > > > > > > > > > > > > > >
On 04/22/2014 10:26 AM, Sebastian Ott wrote: > On Sat, 19 Apr 2014, Benjamin Herrenschmidt wrote: >> On Fri, 2014-04-18 at 12:10 +0200, Sebastian Ott wrote: >>> On Fri, 18 Apr 2014, Benjamin Herrenschmidt wrote: >>>> On Thu, 2014-04-17 at 19:46 +0200, Sebastian Ott wrote: >>>> >>>>> pci: move open fabric devspec attribute to pci common code >>>>> >>>>> Move the devspec OF attribute to pci common code's set of device >>>>> attributes since it's not architecture dependent. >>>>> As a side effect microblaze and powerpc no longer need to use >>>>> pcibios_add_platform_entries. >>>>> >>>>> Link: https://lkml.kernel.org/r/alpine.LFD.2.11.1404141101500.1529@denkbrett >>>>> Signed-off-by: Sebastian Ott <sebott@linux.vnet.ibm.com> >>>> >>>> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>>> >>>> (Pending you've compile tested it on powerpc, I'm on vacation and >>>> haven't done it :) >>> >>> I did: >>> make ARCH=powerpc CROSS_COMPILE=powerpc64-4.3.2- g5_defconfig >>> make ARCH=powerpc CROSS_COMPILE=powerpc64-4.3.2- >>> >>> and failed with: >>> CC arch/powerpc/lib/xor_vmx.o >>> In file included from include/linux/list.h:4, >>> from include/linux/preempt.h:10, >>> from arch/powerpc/lib/xor_vmx.c:22: >>> include/linux/types.h:29: error: both ‘unsigned’ and ‘_Bool’ in declaration specifiers >>> cc1: warnings being treated as errors >>> include/linux/types.h:29: error: useless type name in empty declaration >> >> Not sure what's up here, did you try with a more recent compiler ? 4.3 >> is pretty ancient by kernel standards. >> >> You can find cross compilers there: >> >> https://www.kernel.org/pub/tools/crosstool/ >> >> They aren't the newest either but they should work. > > Yes. That did the trick. Hope that you did the same for microblaze or just push the repo/branch with these patches to zero-day testing system and you got the results. Thanks, Michal
On Wed, 23 Apr 2014, Michal Simek wrote: > On 04/22/2014 10:26 AM, Sebastian Ott wrote: > > On Sat, 19 Apr 2014, Benjamin Herrenschmidt wrote: > >> On Fri, 2014-04-18 at 12:10 +0200, Sebastian Ott wrote: > >>> On Fri, 18 Apr 2014, Benjamin Herrenschmidt wrote: > >>>> On Thu, 2014-04-17 at 19:46 +0200, Sebastian Ott wrote: > >>>> > >>>>> pci: move open fabric devspec attribute to pci common code > >>>>> > >>>>> Move the devspec OF attribute to pci common code's set of device > >>>>> attributes since it's not architecture dependent. > >>>>> As a side effect microblaze and powerpc no longer need to use > >>>>> pcibios_add_platform_entries. > >>>>> > >>>>> Link: https://lkml.kernel.org/r/alpine.LFD.2.11.1404141101500.1529@denkbrett > >>>>> Signed-off-by: Sebastian Ott <sebott@linux.vnet.ibm.com> > >>>> > >>>> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > >>>> > >>>> (Pending you've compile tested it on powerpc, I'm on vacation and > >>>> haven't done it :) > >>> > >>> I did: > >>> make ARCH=powerpc CROSS_COMPILE=powerpc64-4.3.2- g5_defconfig > >>> make ARCH=powerpc CROSS_COMPILE=powerpc64-4.3.2- > >>> > >>> and failed with: > >>> CC arch/powerpc/lib/xor_vmx.o > >>> In file included from include/linux/list.h:4, > >>> from include/linux/preempt.h:10, > >>> from arch/powerpc/lib/xor_vmx.c:22: > >>> include/linux/types.h:29: error: both ‘unsigned’ and ‘_Bool’ in declaration specifiers > >>> cc1: warnings being treated as errors > >>> include/linux/types.h:29: error: useless type name in empty declaration > >> > >> Not sure what's up here, did you try with a more recent compiler ? 4.3 > >> is pretty ancient by kernel standards. > >> > >> You can find cross compilers there: > >> > >> https://www.kernel.org/pub/tools/crosstool/ > >> > >> They aren't the newest either but they should work. > > > > Yes. That did the trick. > > Hope that you did the same for microblaze or just push the repo/branch with these > patches to zero-day testing system and you got the results. Yes, I compile-tested microblaze too. Regards, Sebastian > > Thanks, > Michal > > > -- > Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 > w: www.monstr.eu p: +42-0-721842854 > Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ > Maintainer of Linux kernel - Xilinx Zynq ARM architecture > Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform > > >
On Thu, Apr 17, 2014 at 07:46:15PM +0200, Sebastian Ott wrote: > On Mon, 14 Apr 2014, Sebastian Ott wrote: > > On Mon, 14 Apr 2014, Bjorn Helgaas wrote: > > > On Mon, Apr 14, 2014 at 11:03:42AM +0200, Sebastian Ott wrote: > > > > for pci on s390 we currently use pcibios_add_platform_entries to add > > > > some arch specific attributes to pdevs. This has 2 downsides - it will > > > > race with userspace which is triggered by udev events and expecting > > > > these attributes (but that's a theoretical issue). More important to > > > > me is that one cannot use attribute_groups with this. Both issues could > > > > be addressed by using pdev->dev.groups and let the driver core handle > > > > attribute creation. > > > > > > > > So would it be ok if we set pdev->dev.groups in pcibios_add_device? > > > > (It should be since it's not used by pci common code which uses bus_type, > > > > dev_type, and class groups). > > > > > > Hi Sebastian, > > > > > > Sorry, I meant to respond to this earlier, but forgot. This sounds > > > reasonable to me, but Greg can give you a much better answer than I can. > > > > > > Documentation/driver-model/device.txt says the dev->groups pointer > > > should be set before calling device_register(). PCI calls > > > device_initialize() and device_add() instead of using device_register(), > > > and pcibios_add_device() looks like it happens at the right time: > > > > > > pci_scan_root_bus > > > pci_scan_child_bus > > > pci_scan_slot > > > pci_scan_single_device > > > pci_device_add > > > device_initialize > > > pcibios_add_device # <--- > > > device_add > > > pci_bus_add_devices > > > pci_bus_add_device > > > pci_create_sysfs_dev_files > > > pcibios_add_platform_entries # 8d4cd0833107 (benh) > > > device_attach > > > > > > I'm not sure why pci_create_sysfs_dev_files() is done later. It seems > > > like that should be done before device_add() as well. Maybe it's > > > because BARs might not be valid yet (that doesn't seem like a very good > > > excuse, but it's all I can think of). > > > > > > I assume that if you change s390, you'll also change microblaze and > > > powerpc? They look structurally similar to s390. > > > > Yes, that sounds like a plan - this way we can get rid of > > pcibios_add_platform_entries altogether. I'll send these patches soon. > > Hm, pcibios_add_platform_entries for microblaze and power is identical > and this OF stuff seems not to be arch specific. How about the following > patch? > > > pci: move open fabric devspec attribute to pci common code > > Move the devspec OF attribute to pci common code's set of device > attributes since it's not architecture dependent. > As a side effect microblaze and powerpc no longer need to use > pcibios_add_platform_entries. > > Link: https://lkml.kernel.org/r/alpine.LFD.2.11.1404141101500.1529@denkbrett > Signed-off-by: Sebastian Ott <sebott@linux.vnet.ibm.com> I applied this with Ben's ack to pci/misc for v3.16. I don't see a corresponding s390 patch; did I miss it? I don't want to apply the "Remove pcibios_add_platform_entries()" patch until s390 is fixed up too. Bjorn > --- > arch/microblaze/pci/pci-common.c | 20 -------------------- > arch/powerpc/kernel/pci-common.c | 20 -------------------- > drivers/pci/pci-sysfs.c | 17 +++++++++++++++++ > 3 files changed, 17 insertions(+), 40 deletions(-) > > --- a/arch/microblaze/pci/pci-common.c > +++ b/arch/microblaze/pci/pci-common.c > @@ -168,26 +168,6 @@ struct pci_controller *pci_find_hose_for > return NULL; > } > > -static ssize_t pci_show_devspec(struct device *dev, > - struct device_attribute *attr, char *buf) > -{ > - struct pci_dev *pdev; > - struct device_node *np; > - > - pdev = to_pci_dev(dev); > - np = pci_device_to_OF_node(pdev); > - if (np == NULL || np->full_name == NULL) > - return 0; > - return sprintf(buf, "%s", np->full_name); > -} > -static DEVICE_ATTR(devspec, S_IRUGO, pci_show_devspec, NULL); > - > -/* Add sysfs properties */ > -int pcibios_add_platform_entries(struct pci_dev *pdev) > -{ > - return device_create_file(&pdev->dev, &dev_attr_devspec); > -} > - > void pcibios_set_master(struct pci_dev *dev) > { > /* No special bus mastering setup handling */ > --- a/arch/powerpc/kernel/pci-common.c > +++ b/arch/powerpc/kernel/pci-common.c > @@ -201,26 +201,6 @@ struct pci_controller* pci_find_hose_for > return NULL; > } > > -static ssize_t pci_show_devspec(struct device *dev, > - struct device_attribute *attr, char *buf) > -{ > - struct pci_dev *pdev; > - struct device_node *np; > - > - pdev = to_pci_dev (dev); > - np = pci_device_to_OF_node(pdev); > - if (np == NULL || np->full_name == NULL) > - return 0; > - return sprintf(buf, "%s", np->full_name); > -} > -static DEVICE_ATTR(devspec, S_IRUGO, pci_show_devspec, NULL); > - > -/* Add sysfs properties */ > -int pcibios_add_platform_entries(struct pci_dev *pdev) > -{ > - return device_create_file(&pdev->dev, &dev_attr_devspec); > -} > - > /* > * Reads the interrupt pin to determine if interrupt is use by card. > * If the interrupt is used, then gets the interrupt line from the > --- a/drivers/pci/pci-sysfs.c > +++ b/drivers/pci/pci-sysfs.c > @@ -416,6 +416,20 @@ static ssize_t d3cold_allowed_show(struc > static DEVICE_ATTR_RW(d3cold_allowed); > #endif > > +#ifdef CONFIG_OF > +static ssize_t devspec_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct pci_dev *pdev = to_pci_dev(dev); > + struct device_node *np = pci_device_to_OF_node(pdev); > + > + if (np == NULL || np->full_name == NULL) > + return 0; > + return sprintf(buf, "%s", np->full_name); > +} > +static DEVICE_ATTR_RO(devspec); > +#endif > + > #ifdef CONFIG_PCI_IOV > static ssize_t sriov_totalvfs_show(struct device *dev, > struct device_attribute *attr, > @@ -521,6 +535,9 @@ static struct attribute *pci_dev_attrs[] > #if defined(CONFIG_PM_RUNTIME) && defined(CONFIG_ACPI) > &dev_attr_d3cold_allowed.attr, > #endif > +#ifdef CONFIG_OF > + &dev_attr_devspec.attr, > +#endif > NULL, > }; > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/arch/microblaze/pci/pci-common.c +++ b/arch/microblaze/pci/pci-common.c @@ -168,26 +168,6 @@ struct pci_controller *pci_find_hose_for return NULL; } -static ssize_t pci_show_devspec(struct device *dev, - struct device_attribute *attr, char *buf) -{ - struct pci_dev *pdev; - struct device_node *np; - - pdev = to_pci_dev(dev); - np = pci_device_to_OF_node(pdev); - if (np == NULL || np->full_name == NULL) - return 0; - return sprintf(buf, "%s", np->full_name); -} -static DEVICE_ATTR(devspec, S_IRUGO, pci_show_devspec, NULL); - -/* Add sysfs properties */ -int pcibios_add_platform_entries(struct pci_dev *pdev) -{ - return device_create_file(&pdev->dev, &dev_attr_devspec); -} - void pcibios_set_master(struct pci_dev *dev) { /* No special bus mastering setup handling */ --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -201,26 +201,6 @@ struct pci_controller* pci_find_hose_for return NULL; } -static ssize_t pci_show_devspec(struct device *dev, - struct device_attribute *attr, char *buf) -{ - struct pci_dev *pdev; - struct device_node *np; - - pdev = to_pci_dev (dev); - np = pci_device_to_OF_node(pdev); - if (np == NULL || np->full_name == NULL) - return 0; - return sprintf(buf, "%s", np->full_name); -} -static DEVICE_ATTR(devspec, S_IRUGO, pci_show_devspec, NULL); - -/* Add sysfs properties */ -int pcibios_add_platform_entries(struct pci_dev *pdev) -{ - return device_create_file(&pdev->dev, &dev_attr_devspec); -} - /* * Reads the interrupt pin to determine if interrupt is use by card. * If the interrupt is used, then gets the interrupt line from the --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c @@ -416,6 +416,20 @@ static ssize_t d3cold_allowed_show(struc static DEVICE_ATTR_RW(d3cold_allowed); #endif +#ifdef CONFIG_OF +static ssize_t devspec_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct pci_dev *pdev = to_pci_dev(dev); + struct device_node *np = pci_device_to_OF_node(pdev); + + if (np == NULL || np->full_name == NULL) + return 0; + return sprintf(buf, "%s", np->full_name); +} +static DEVICE_ATTR_RO(devspec); +#endif + #ifdef CONFIG_PCI_IOV static ssize_t sriov_totalvfs_show(struct device *dev, struct device_attribute *attr, @@ -521,6 +535,9 @@ static struct attribute *pci_dev_attrs[] #if defined(CONFIG_PM_RUNTIME) && defined(CONFIG_ACPI) &dev_attr_d3cold_allowed.attr, #endif +#ifdef CONFIG_OF + &dev_attr_devspec.attr, +#endif NULL, };