From: "Pali Rohár" <pali@kernel.org> To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: "Ray Jui" <ray.jui@broadcom.com>, "Roman Bacik" <roman.bacik@broadcom.com>, "Rob Herring" <robh@kernel.org>, "Krzysztof Wilczyński" <kw@linux.com>, "Bjorn Helgaas" <bhelgaas@google.com>, bcm-kernel-feedback-list@broadcom.com, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] PCI: iproc: Set all 24 bits of PCI class code Date: Mon, 14 Feb 2022 12:49:10 +0100 [thread overview] Message-ID: <20220214114910.vaez3coyyfztabni@pali> (raw) In-Reply-To: <20220211162317.GC448@lpieralisi> On Friday 11 February 2022 16:23:17 Lorenzo Pieralisi wrote: > On Wed, Jan 05, 2022 at 07:13:06PM +0100, Pali Rohár wrote: > > Hello! > > > > On Wednesday 05 January 2022 09:51:48 Ray Jui wrote: > > > Hi Pali, > > > > > > On 1/5/2022 1:35 AM, Pali Rohár wrote: > > > > Register 0x43c in its low 24 bits contains PCI class code. > > > > > > > > Update code to set all 24 bits of PCI class code and not only upper 16 bits > > > > of PCI class code. > > > > > > > > Use a new macro PCI_CLASS_BRIDGE_PCI_NORMAL which represents whole 24 bits > > > > of normal PCI bridge class. > > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > > > > > --- > > > > Roman helped me with this change and confirmed that class code is stored > > > > really in bits [23:0] of custom register 0x43c (normally class code is > > > > stored in bits [31:8] of pci register 0x08). > > > > > > > > This patch depends on patch which adds PCI_CLASS_BRIDGE_PCI_NORMAL macro: > > > > https://lore.kernel.org/linux-pci/20211220145140.31898-1-pali@kernel.org/ > > > > --- > > > > drivers/pci/controller/pcie-iproc.c | 9 ++++----- > > > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c > > > > index 3df4ab209253..2519201b0e51 100644 > > > > --- a/drivers/pci/controller/pcie-iproc.c > > > > +++ b/drivers/pci/controller/pcie-iproc.c > > > > @@ -789,14 +789,13 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie) > > > > return -EFAULT; > > > > } > > > > > > > > - /* force class to PCI_CLASS_BRIDGE_PCI (0x0604) */ > > > > + /* force class to PCI_CLASS_BRIDGE_PCI_NORMAL (0x060400) */ > > > > #define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c > > > > -#define PCI_CLASS_BRIDGE_MASK 0xffff00 > > > > -#define PCI_CLASS_BRIDGE_SHIFT 8 > > > > +#define PCI_BRIDGE_CTRL_REG_CLASS_MASK 0xffffff > > > > iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, > > > > 4, &class); > > > > - class &= ~PCI_CLASS_BRIDGE_MASK; > > > > - class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT); > > > > + class &= ~PCI_BRIDGE_CTRL_REG_CLASS_MASK; > > > > + class |= PCI_CLASS_BRIDGE_PCI_NORMAL; > > > > iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, > > > > 4, class); > > > > > > > > > > I have two comments: > > > > > > 1. You do not seem to generate the email list using the > > > get_maintainer.pl script, so the two maintainers for Broadcom ARM > > > architecture (Ray Jui and Scott Branden) are left out. > > > > Ou, sorry for that! I have generated this patch for U-Boot and Linux > > kernel and probably mixed or forgot to include correct recipients for > > correct project. > > > > > 2. I suppose 'PCI_CLASS_BRIDGE_PCI_NORMAL' is defined in some common PCI > > > header in a separate patch as described in the commit message. Then how > > > come these patches are not constructed with a patch series? > > > > Yes, PCI_CLASS_BRIDGE_PCI_NORMAL is a new constant for common pci header > > file defined in patch linked in commit message. > > https://lore.kernel.org/linux-pci/20211220145140.31898-1-pali@kernel.org/ > > > > Originally I included this change in v1 of linked patch in December but > > I realized that it does not match standard PCI config space (different > > offset 0x43c vs 0x08 and also different shift 0x8 vs 0x0) and probably > > there is something either incorrect or really non-standard. So later in > > December I dropped iproc_pcie_check_link() change in v2 of the linked > > patch where is introduced PCI_CLASS_BRIDGE_PCI_NORMAL and now sent new > > change for iproc_pcie_check_link() separately. > > > > Technically, linked patch in commit message is just extracting code into > > the common macros without any functional changed. But change in this > > iproc_pcie_check_link() has also functional change as now also lower 8 > > bits of class code are changed. So in my opinion this patch should be > > really separate of linked patch. > > > > I hope that Lorenzo and Bjorn take patches in correct order... > > Can you resend the patches in a series please, I will drop this one. Done! https://lore.kernel.org/linux-pci/20220214114109.26809-2-pali@kernel.org/T/#u > Thanks, > Lorenzo > > > > Other than, the change itself is exactly what I sent to Roman and looks > > > good to me. Thanks. > > > > > > Acked-by: Ray Jui <ray.jui@broadcom.com> > > > > Perfect!
WARNING: multiple messages have this Message-ID (diff)
From: "Pali Rohár" <pali@kernel.org> To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: "Ray Jui" <ray.jui@broadcom.com>, "Roman Bacik" <roman.bacik@broadcom.com>, "Rob Herring" <robh@kernel.org>, "Krzysztof Wilczyński" <kw@linux.com>, "Bjorn Helgaas" <bhelgaas@google.com>, bcm-kernel-feedback-list@broadcom.com, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] PCI: iproc: Set all 24 bits of PCI class code Date: Mon, 14 Feb 2022 12:49:10 +0100 [thread overview] Message-ID: <20220214114910.vaez3coyyfztabni@pali> (raw) In-Reply-To: <20220211162317.GC448@lpieralisi> On Friday 11 February 2022 16:23:17 Lorenzo Pieralisi wrote: > On Wed, Jan 05, 2022 at 07:13:06PM +0100, Pali Rohár wrote: > > Hello! > > > > On Wednesday 05 January 2022 09:51:48 Ray Jui wrote: > > > Hi Pali, > > > > > > On 1/5/2022 1:35 AM, Pali Rohár wrote: > > > > Register 0x43c in its low 24 bits contains PCI class code. > > > > > > > > Update code to set all 24 bits of PCI class code and not only upper 16 bits > > > > of PCI class code. > > > > > > > > Use a new macro PCI_CLASS_BRIDGE_PCI_NORMAL which represents whole 24 bits > > > > of normal PCI bridge class. > > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > > > > > --- > > > > Roman helped me with this change and confirmed that class code is stored > > > > really in bits [23:0] of custom register 0x43c (normally class code is > > > > stored in bits [31:8] of pci register 0x08). > > > > > > > > This patch depends on patch which adds PCI_CLASS_BRIDGE_PCI_NORMAL macro: > > > > https://lore.kernel.org/linux-pci/20211220145140.31898-1-pali@kernel.org/ > > > > --- > > > > drivers/pci/controller/pcie-iproc.c | 9 ++++----- > > > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c > > > > index 3df4ab209253..2519201b0e51 100644 > > > > --- a/drivers/pci/controller/pcie-iproc.c > > > > +++ b/drivers/pci/controller/pcie-iproc.c > > > > @@ -789,14 +789,13 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie) > > > > return -EFAULT; > > > > } > > > > > > > > - /* force class to PCI_CLASS_BRIDGE_PCI (0x0604) */ > > > > + /* force class to PCI_CLASS_BRIDGE_PCI_NORMAL (0x060400) */ > > > > #define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c > > > > -#define PCI_CLASS_BRIDGE_MASK 0xffff00 > > > > -#define PCI_CLASS_BRIDGE_SHIFT 8 > > > > +#define PCI_BRIDGE_CTRL_REG_CLASS_MASK 0xffffff > > > > iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, > > > > 4, &class); > > > > - class &= ~PCI_CLASS_BRIDGE_MASK; > > > > - class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT); > > > > + class &= ~PCI_BRIDGE_CTRL_REG_CLASS_MASK; > > > > + class |= PCI_CLASS_BRIDGE_PCI_NORMAL; > > > > iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, > > > > 4, class); > > > > > > > > > > I have two comments: > > > > > > 1. You do not seem to generate the email list using the > > > get_maintainer.pl script, so the two maintainers for Broadcom ARM > > > architecture (Ray Jui and Scott Branden) are left out. > > > > Ou, sorry for that! I have generated this patch for U-Boot and Linux > > kernel and probably mixed or forgot to include correct recipients for > > correct project. > > > > > 2. I suppose 'PCI_CLASS_BRIDGE_PCI_NORMAL' is defined in some common PCI > > > header in a separate patch as described in the commit message. Then how > > > come these patches are not constructed with a patch series? > > > > Yes, PCI_CLASS_BRIDGE_PCI_NORMAL is a new constant for common pci header > > file defined in patch linked in commit message. > > https://lore.kernel.org/linux-pci/20211220145140.31898-1-pali@kernel.org/ > > > > Originally I included this change in v1 of linked patch in December but > > I realized that it does not match standard PCI config space (different > > offset 0x43c vs 0x08 and also different shift 0x8 vs 0x0) and probably > > there is something either incorrect or really non-standard. So later in > > December I dropped iproc_pcie_check_link() change in v2 of the linked > > patch where is introduced PCI_CLASS_BRIDGE_PCI_NORMAL and now sent new > > change for iproc_pcie_check_link() separately. > > > > Technically, linked patch in commit message is just extracting code into > > the common macros without any functional changed. But change in this > > iproc_pcie_check_link() has also functional change as now also lower 8 > > bits of class code are changed. So in my opinion this patch should be > > really separate of linked patch. > > > > I hope that Lorenzo and Bjorn take patches in correct order... > > Can you resend the patches in a series please, I will drop this one. Done! https://lore.kernel.org/linux-pci/20220214114109.26809-2-pali@kernel.org/T/#u > Thanks, > Lorenzo > > > > Other than, the change itself is exactly what I sent to Roman and looks > > > good to me. Thanks. > > > > > > Acked-by: Ray Jui <ray.jui@broadcom.com> > > > > Perfect! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-02-14 11:50 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-01-05 9:35 [PATCH] PCI: iproc: Set all 24 bits of PCI class code Pali Rohár 2022-01-05 9:35 ` Pali Rohár 2022-01-05 14:16 ` kernel test robot 2022-01-05 14:16 ` kernel test robot 2022-01-05 14:16 ` kernel test robot 2022-01-05 16:57 ` Roman Bacik 2022-01-05 16:57 ` Roman Bacik 2022-01-05 17:51 ` Ray Jui 2022-01-05 17:51 ` Ray Jui 2022-01-05 18:13 ` Pali Rohár 2022-01-05 18:13 ` Pali Rohár 2022-01-06 18:00 ` Bjorn Helgaas 2022-01-06 18:00 ` Bjorn Helgaas 2022-01-07 11:43 ` Lorenzo Pieralisi 2022-01-07 11:43 ` Lorenzo Pieralisi 2022-02-11 16:23 ` Lorenzo Pieralisi 2022-02-11 16:23 ` Lorenzo Pieralisi 2022-02-14 11:49 ` Pali Rohár [this message] 2022-02-14 11:49 ` Pali Rohár 2022-01-05 21:05 ` kernel test robot 2022-01-05 21:05 ` kernel test robot 2022-01-05 21:05 ` kernel test robot
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220214114910.vaez3coyyfztabni@pali \ --to=pali@kernel.org \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=bhelgaas@google.com \ --cc=kw@linux.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=ray.jui@broadcom.com \ --cc=robh@kernel.org \ --cc=roman.bacik@broadcom.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.