| Message ID | 20190118114642.19412-2-sr@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [U-Boot] pci: Add PCI_SLOT macro to include/pci.h | expand |
Hi Stefan, On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr@denx.de> wrote: > > This macro will be used the by the Marvell Armada XP/38x PCIe driver, > which is moved to DM right now. > > Signed-off-by: Stefan Roese <sr@denx.de> > Cc: Bin Meng <bmeng.cn@gmail.com> > Cc: Simon Glass <sjg@chromium.org> > --- > include/pci.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/pci.h b/include/pci.h > index 785d7d28b7..f4a9e025b3 100644 > --- a/include/pci.h > +++ b/include/pci.h > @@ -501,6 +501,7 @@ typedef int pci_dev_t; > #define PCI_BUS(d) (((d) >> 16) & 0xff) > #define PCI_DEV(d) (((d) >> 11) & 0x1f) > #define PCI_FUNC(d) (((d) >> 8) & 0x7) > +#define PCI_SLOT(d) (((d) >> 3) & 0x1f) This seems unrelated to the other macros, since is shifts left only 3 positions. Can you perhaps move it to the end and add a comment as to what the input is and what it returns? It seems different to the others. > #define PCI_DEVFN(d, f) ((d) << 11 | (f) << 8) > #define PCI_MASK_BUS(bdf) ((bdf) & 0xffff) > #define PCI_ADD_BUS(bus, devfn) (((bus) << 16) | (devfn)) > -- > 2.20.1 > Regards, Simon
Hi Stefan, On Tue, Jan 22, 2019 at 8:32 AM Simon Glass <sjg@chromium.org> wrote: > > Hi Stefan, > > On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr@denx.de> wrote: > > > > This macro will be used the by the Marvell Armada XP/38x PCIe driver, > > which is moved to DM right now. > > > > Signed-off-by: Stefan Roese <sr@denx.de> > > Cc: Bin Meng <bmeng.cn@gmail.com> > > Cc: Simon Glass <sjg@chromium.org> > > --- It's weird I did not receive this email in my inbox. > > include/pci.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/include/pci.h b/include/pci.h > > index 785d7d28b7..f4a9e025b3 100644 > > --- a/include/pci.h > > +++ b/include/pci.h > > @@ -501,6 +501,7 @@ typedef int pci_dev_t; > > #define PCI_BUS(d) (((d) >> 16) & 0xff) > > #define PCI_DEV(d) (((d) >> 11) & 0x1f) > > #define PCI_FUNC(d) (((d) >> 8) & 0x7) > > +#define PCI_SLOT(d) (((d) >> 3) & 0x1f) > > This seems unrelated to the other macros, since is shifts left only 3 > positions. Can you perhaps move it to the end and add a comment as to > what the input is and what it returns? It seems different to the > others. Agreed with Simon. Do you have any follow-up patches that will use this macro for better understanding? > > > #define PCI_DEVFN(d, f) ((d) << 11 | (f) << 8) > > #define PCI_MASK_BUS(bdf) ((bdf) & 0xffff) > > #define PCI_ADD_BUS(bus, devfn) (((bus) << 16) | (devfn)) > > -- Regards, Bin
Hi Simon, On 22.01.19 01:32, Simon Glass wrote: > On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr@denx.de> wrote: >> >> This macro will be used the by the Marvell Armada XP/38x PCIe driver, >> which is moved to DM right now. >> >> Signed-off-by: Stefan Roese <sr@denx.de> >> Cc: Bin Meng <bmeng.cn@gmail.com> >> Cc: Simon Glass <sjg@chromium.org> >> --- >> include/pci.h | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/include/pci.h b/include/pci.h >> index 785d7d28b7..f4a9e025b3 100644 >> --- a/include/pci.h >> +++ b/include/pci.h >> @@ -501,6 +501,7 @@ typedef int pci_dev_t; >> #define PCI_BUS(d) (((d) >> 16) & 0xff) >> #define PCI_DEV(d) (((d) >> 11) & 0x1f) >> #define PCI_FUNC(d) (((d) >> 8) & 0x7) >> +#define PCI_SLOT(d) (((d) >> 3) & 0x1f) > > This seems unrelated to the other macros, since is shifts left only 3 > positions. Can you perhaps move it to the end and add a comment as to > what the input is and what it returns? It seems different to the > others. We seem to have the same different interpretation (U-Boot vs Linux) as remarked in my last mail. It seems that U-Boot expects devfn to reside in bits 15-8. Regarding the input and what it returns: My current understanding is, that PCI_DEV in U-Boot is identical to PCI_SLOT in Linux (PCI_DEV is not used there at all). The input here is devfn as well. I can adapt my driver to use PCI_DEV instead. But frankly, these different macro implementations (U-Boot vs Linux) are confusing. Thanks, Stefan
Hi Bin, On 22.01.19 03:11, Bin Meng wrote: > On Tue, Jan 22, 2019 at 8:32 AM Simon Glass <sjg@chromium.org> wrote: >> >> Hi Stefan, >> >> On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr@denx.de> wrote: >>> >>> This macro will be used the by the Marvell Armada XP/38x PCIe driver, >>> which is moved to DM right now. >>> >>> Signed-off-by: Stefan Roese <sr@denx.de> >>> Cc: Bin Meng <bmeng.cn@gmail.com> >>> Cc: Simon Glass <sjg@chromium.org> >>> --- > > It's weird I did not receive this email in my inbox. Hmm, strange. >>> include/pci.h | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/include/pci.h b/include/pci.h >>> index 785d7d28b7..f4a9e025b3 100644 >>> --- a/include/pci.h >>> +++ b/include/pci.h >>> @@ -501,6 +501,7 @@ typedef int pci_dev_t; >>> #define PCI_BUS(d) (((d) >> 16) & 0xff) >>> #define PCI_DEV(d) (((d) >> 11) & 0x1f) >>> #define PCI_FUNC(d) (((d) >> 8) & 0x7) >>> +#define PCI_SLOT(d) (((d) >> 3) & 0x1f) >> >> This seems unrelated to the other macros, since is shifts left only 3 >> positions. Can you perhaps move it to the end and add a comment as to >> what the input is and what it returns? It seems different to the >> others. > > Agreed with Simon. Do you have any follow-up patches that will use > this macro for better understanding? Please see my comments on the last 2 mails and this PCI driver move to DM: http://patchwork.ozlabs.org/patch/1027268/ Thanks, Stefan
diff --git a/include/pci.h b/include/pci.h index 785d7d28b7..f4a9e025b3 100644 --- a/include/pci.h +++ b/include/pci.h @@ -501,6 +501,7 @@ typedef int pci_dev_t; #define PCI_BUS(d) (((d) >> 16) & 0xff) #define PCI_DEV(d) (((d) >> 11) & 0x1f) #define PCI_FUNC(d) (((d) >> 8) & 0x7) +#define PCI_SLOT(d) (((d) >> 3) & 0x1f) #define PCI_DEVFN(d, f) ((d) << 11 | (f) << 8) #define PCI_MASK_BUS(bdf) ((bdf) & 0xffff) #define PCI_ADD_BUS(bus, devfn) (((bus) << 16) | (devfn))
This macro will be used the by the Marvell Armada XP/38x PCIe driver, which is moved to DM right now. Signed-off-by: Stefan Roese <sr@denx.de> Cc: Bin Meng <bmeng.cn@gmail.com> Cc: Simon Glass <sjg@chromium.org> --- include/pci.h | 1 + 1 file changed, 1 insertion(+)