mbox series

[0/2] PCI: aardvark: Fix comphy with old ATF

Message ID 20200902144344.16684-1-pali@kernel.org
Headers show
Series PCI: aardvark: Fix comphy with old ATF | expand

Message

Pali Rohár Sept. 2, 2020, 2:43 p.m. UTC
This patch series fixes regression introduced in commit 366697018c9a
("PCI: aardvark: Add PHY support") which caused aardvark driver
initialization failure on EspressoBin board with factory version of
Arm Trusted Firmware provided by Marvell.

Second patch depends on the first patch, so please add appropriate
Fixes/Cc:stable@ tags to have both patches correctly backported to
stable kernels.

I have tested both patches with Marvell ATF firmware ebin-17.10-uart.zip
and with upstream ATF+uboot and aardvark was initialized successfully.
Without this patch series on ebin-17.10-uart.zip aardvark initialization
failed.

Pali Rohár (2):
  phy: marvell: comphy: Convert internal SMCC firmware return codes to
    errno
  PCI: aardvark: Fix initialization with old Marvell's Arm Trusted
    Firmware

 drivers/pci/controller/pci-aardvark.c        |  4 +++-
 drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 14 +++++++++++---
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 14 +++++++++++---
 3 files changed, 25 insertions(+), 7 deletions(-)

Comments

Tomasz Maciej Nowak Sept. 16, 2020, 3:14 p.m. UTC | #1
W dniu 02.09.2020 o 16:43, Pali Rohár pisze:
> This patch series fixes regression introduced in commit 366697018c9a
> ("PCI: aardvark: Add PHY support") which caused aardvark driver
> initialization failure on EspressoBin board with factory version of
> Arm Trusted Firmware provided by Marvell.
> 
> Second patch depends on the first patch, so please add appropriate
> Fixes/Cc:stable@ tags to have both patches correctly backported to
> stable kernels.
> 
> I have tested both patches with Marvell ATF firmware ebin-17.10-uart.zip
> and with upstream ATF+uboot and aardvark was initialized successfully.
> Without this patch series on ebin-17.10-uart.zip aardvark initialization
> failed.
> 
> Pali Rohár (2):
>   phy: marvell: comphy: Convert internal SMCC firmware return codes to
>     errno
>   PCI: aardvark: Fix initialization with old Marvell's Arm Trusted
>     Firmware

For both patches

Tested-by: Tomasz Maciej Nowak <tmn505@gmail.com>

> 
>  drivers/pci/controller/pci-aardvark.c        |  4 +++-
>  drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 14 +++++++++++---
>  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 14 +++++++++++---
>  3 files changed, 25 insertions(+), 7 deletions(-)
>
Pali Rohár Sept. 21, 2020, 1:09 p.m. UTC | #2
Lorenzo, could you look and review this patch series? It fixes commit
366697018c9a and we would like to have fix also in stable kernels.

On Wednesday 16 September 2020 17:14:02 Tomasz Maciej Nowak wrote:
> W dniu 02.09.2020 o 16:43, Pali Rohár pisze:
> > This patch series fixes regression introduced in commit 366697018c9a
> > ("PCI: aardvark: Add PHY support") which caused aardvark driver
> > initialization failure on EspressoBin board with factory version of
> > Arm Trusted Firmware provided by Marvell.
> > 
> > Second patch depends on the first patch, so please add appropriate
> > Fixes/Cc:stable@ tags to have both patches correctly backported to
> > stable kernels.
> > 
> > I have tested both patches with Marvell ATF firmware ebin-17.10-uart.zip
> > and with upstream ATF+uboot and aardvark was initialized successfully.
> > Without this patch series on ebin-17.10-uart.zip aardvark initialization
> > failed.
> > 
> > Pali Rohár (2):
> >   phy: marvell: comphy: Convert internal SMCC firmware return codes to
> >     errno
> >   PCI: aardvark: Fix initialization with old Marvell's Arm Trusted
> >     Firmware
> 
> For both patches
> 
> Tested-by: Tomasz Maciej Nowak <tmn505@gmail.com>
> 
> > 
> >  drivers/pci/controller/pci-aardvark.c        |  4 +++-
> >  drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 14 +++++++++++---
> >  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 14 +++++++++++---
> >  3 files changed, 25 insertions(+), 7 deletions(-)
> > 
>
Pali Rohár Oct. 2, 2020, noon UTC | #3
On Wednesday 02 September 2020 16:43:42 Pali Rohár wrote:
> This patch series fixes regression introduced in commit 366697018c9a
> ("PCI: aardvark: Add PHY support") which caused aardvark driver
> initialization failure on EspressoBin board with factory version of
> Arm Trusted Firmware provided by Marvell.
> 
> Second patch depends on the first patch, so please add appropriate
> Fixes/Cc:stable@ tags to have both patches correctly backported to
> stable kernels.
> 
> I have tested both patches with Marvell ATF firmware ebin-17.10-uart.zip
> and with upstream ATF+uboot and aardvark was initialized successfully.
> Without this patch series on ebin-17.10-uart.zip aardvark initialization
> failed.

Lorenzo or Bjorn, could you review this patch series? I would like to
see this regression fixed in stable kernels.

> Pali Rohár (2):
>   phy: marvell: comphy: Convert internal SMCC firmware return codes to
>     errno
>   PCI: aardvark: Fix initialization with old Marvell's Arm Trusted
>     Firmware
> 
>  drivers/pci/controller/pci-aardvark.c        |  4 +++-
>  drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 14 +++++++++++---
>  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 14 +++++++++++---
>  3 files changed, 25 insertions(+), 7 deletions(-)
> 
> -- 
> 2.20.1
>
Lorenzo Pieralisi Oct. 2, 2020, 1:37 p.m. UTC | #4
On Wed, Sep 02, 2020 at 04:43:42PM +0200, Pali Rohár wrote:
> This patch series fixes regression introduced in commit 366697018c9a
> ("PCI: aardvark: Add PHY support") which caused aardvark driver
> initialization failure on EspressoBin board with factory version of
> Arm Trusted Firmware provided by Marvell.
> 
> Second patch depends on the first patch, so please add appropriate
> Fixes/Cc:stable@ tags to have both patches correctly backported to
> stable kernels.
> 
> I have tested both patches with Marvell ATF firmware ebin-17.10-uart.zip
> and with upstream ATF+uboot and aardvark was initialized successfully.
> Without this patch series on ebin-17.10-uart.zip aardvark initialization
> failed.
> 
> Pali Rohár (2):
>   phy: marvell: comphy: Convert internal SMCC firmware return codes to
>     errno
>   PCI: aardvark: Fix initialization with old Marvell's Arm Trusted
>     Firmware
> 
>  drivers/pci/controller/pci-aardvark.c        |  4 +++-
>  drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 14 +++++++++++---
>  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 14 +++++++++++---
>  3 files changed, 25 insertions(+), 7 deletions(-)

Applied to pci/aardvark (both patches), thanks.

Lorenzo
Pali Rohár Oct. 2, 2020, 2:26 p.m. UTC | #5
On Friday 02 October 2020 14:37:13 Lorenzo Pieralisi wrote:
> On Wed, Sep 02, 2020 at 04:43:42PM +0200, Pali Rohár wrote:
> > This patch series fixes regression introduced in commit 366697018c9a
> > ("PCI: aardvark: Add PHY support") which caused aardvark driver
> > initialization failure on EspressoBin board with factory version of
> > Arm Trusted Firmware provided by Marvell.
> > 
> > Second patch depends on the first patch, so please add appropriate
> > Fixes/Cc:stable@ tags to have both patches correctly backported to
> > stable kernels.
> > 
> > I have tested both patches with Marvell ATF firmware ebin-17.10-uart.zip
> > and with upstream ATF+uboot and aardvark was initialized successfully.
> > Without this patch series on ebin-17.10-uart.zip aardvark initialization
> > failed.
> > 
> > Pali Rohár (2):
> >   phy: marvell: comphy: Convert internal SMCC firmware return codes to
> >     errno
> >   PCI: aardvark: Fix initialization with old Marvell's Arm Trusted
> >     Firmware
> > 
> >  drivers/pci/controller/pci-aardvark.c        |  4 +++-
> >  drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 14 +++++++++++---
> >  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 14 +++++++++++---
> >  3 files changed, 25 insertions(+), 7 deletions(-)
> 
> Applied to pci/aardvark (both patches), thanks.

Ok! For second patch would be needed to put CC:stable line with
specifying prerequisite of first patch as written in document:

https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

If I understood it correctly, second patch needs following line:

Cc: <stable@vger.kernel.org> # <commit_id_of_first_path>: comphy: errno return codes

where <commit_id_of_first_path> is commit id of PATCH 1/2.

(correct me if I'm wrong)

Now when first commit has commit id, could you update second commit to
include this information about prerequisite?
Lorenzo Pieralisi Oct. 2, 2020, 2:38 p.m. UTC | #6
On Fri, Oct 02, 2020 at 04:26:16PM +0200, Pali Rohár wrote:
> On Friday 02 October 2020 14:37:13 Lorenzo Pieralisi wrote:
> > On Wed, Sep 02, 2020 at 04:43:42PM +0200, Pali Rohár wrote:
> > > This patch series fixes regression introduced in commit 366697018c9a
> > > ("PCI: aardvark: Add PHY support") which caused aardvark driver
> > > initialization failure on EspressoBin board with factory version of
> > > Arm Trusted Firmware provided by Marvell.
> > > 
> > > Second patch depends on the first patch, so please add appropriate
> > > Fixes/Cc:stable@ tags to have both patches correctly backported to
> > > stable kernels.
> > > 
> > > I have tested both patches with Marvell ATF firmware ebin-17.10-uart.zip
> > > and with upstream ATF+uboot and aardvark was initialized successfully.
> > > Without this patch series on ebin-17.10-uart.zip aardvark initialization
> > > failed.
> > > 
> > > Pali Rohár (2):
> > >   phy: marvell: comphy: Convert internal SMCC firmware return codes to
> > >     errno
> > >   PCI: aardvark: Fix initialization with old Marvell's Arm Trusted
> > >     Firmware
> > > 
> > >  drivers/pci/controller/pci-aardvark.c        |  4 +++-
> > >  drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 14 +++++++++++---
> > >  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 14 +++++++++++---
> > >  3 files changed, 25 insertions(+), 7 deletions(-)
> > 
> > Applied to pci/aardvark (both patches), thanks.
> 
> Ok! For second patch would be needed to put CC:stable line with
> specifying prerequisite of first patch as written in document:
> 
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> 
> If I understood it correctly, second patch needs following line:
> 
> Cc: <stable@vger.kernel.org> # <commit_id_of_first_path>: comphy: errno return codes
> 
> where <commit_id_of_first_path> is commit id of PATCH 1/2.
> 
> (correct me if I'm wrong)
> 
> Now when first commit has commit id, could you update second commit to
> include this information about prerequisite?

No I can't because they will be merged at the same time. What we can
do is mark the second patch for stable and during the stable review
ask to pull patch (1). Or better you shall send both patches to stable
kernels when they hit Linus' tree.

Lorenzo
Pali Rohár Oct. 2, 2020, 2:52 p.m. UTC | #7
On Friday 02 October 2020 15:38:51 Lorenzo Pieralisi wrote:
> On Fri, Oct 02, 2020 at 04:26:16PM +0200, Pali Rohár wrote:
> > On Friday 02 October 2020 14:37:13 Lorenzo Pieralisi wrote:
> > > On Wed, Sep 02, 2020 at 04:43:42PM +0200, Pali Rohár wrote:
> > > > This patch series fixes regression introduced in commit 366697018c9a
> > > > ("PCI: aardvark: Add PHY support") which caused aardvark driver
> > > > initialization failure on EspressoBin board with factory version of
> > > > Arm Trusted Firmware provided by Marvell.
> > > > 
> > > > Second patch depends on the first patch, so please add appropriate
> > > > Fixes/Cc:stable@ tags to have both patches correctly backported to
> > > > stable kernels.
> > > > 
> > > > I have tested both patches with Marvell ATF firmware ebin-17.10-uart.zip
> > > > and with upstream ATF+uboot and aardvark was initialized successfully.
> > > > Without this patch series on ebin-17.10-uart.zip aardvark initialization
> > > > failed.
> > > > 
> > > > Pali Rohár (2):
> > > >   phy: marvell: comphy: Convert internal SMCC firmware return codes to
> > > >     errno
> > > >   PCI: aardvark: Fix initialization with old Marvell's Arm Trusted
> > > >     Firmware
> > > > 
> > > >  drivers/pci/controller/pci-aardvark.c        |  4 +++-
> > > >  drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 14 +++++++++++---
> > > >  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 14 +++++++++++---
> > > >  3 files changed, 25 insertions(+), 7 deletions(-)
> > > 
> > > Applied to pci/aardvark (both patches), thanks.
> > 
> > Ok! For second patch would be needed to put CC:stable line with
> > specifying prerequisite of first patch as written in document:
> > 
> > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > 
> > If I understood it correctly, second patch needs following line:
> > 
> > Cc: <stable@vger.kernel.org> # <commit_id_of_first_path>: comphy: errno return codes
> > 
> > where <commit_id_of_first_path> is commit id of PATCH 1/2.
> > 
> > (correct me if I'm wrong)
> > 
> > Now when first commit has commit id, could you update second commit to
> > include this information about prerequisite?
> 
> No I can't because they will be merged at the same time.

And it is a problem? Git commit id of first patch would not be changed,
so referencing to it is now possible from second commit (unless you do
rebasing).

> What we can do is mark the second patch for stable

This is done by adding "Cc: <stable@vger.kernel.org>" line to second
patch which I suggested, right?

> and during the stable review
> ask to pull patch (1). Or better you shall send both patches to stable
> kernels when they hit Linus' tree.
> 
> Lorenzo
Lorenzo Pieralisi Oct. 2, 2020, 3:03 p.m. UTC | #8
On Fri, Oct 02, 2020 at 04:52:37PM +0200, Pali Rohár wrote:
> On Friday 02 October 2020 15:38:51 Lorenzo Pieralisi wrote:
> > On Fri, Oct 02, 2020 at 04:26:16PM +0200, Pali Rohár wrote:
> > > On Friday 02 October 2020 14:37:13 Lorenzo Pieralisi wrote:
> > > > On Wed, Sep 02, 2020 at 04:43:42PM +0200, Pali Rohár wrote:
> > > > > This patch series fixes regression introduced in commit 366697018c9a
> > > > > ("PCI: aardvark: Add PHY support") which caused aardvark driver
> > > > > initialization failure on EspressoBin board with factory version of
> > > > > Arm Trusted Firmware provided by Marvell.
> > > > > 
> > > > > Second patch depends on the first patch, so please add appropriate
> > > > > Fixes/Cc:stable@ tags to have both patches correctly backported to
> > > > > stable kernels.
> > > > > 
> > > > > I have tested both patches with Marvell ATF firmware ebin-17.10-uart.zip
> > > > > and with upstream ATF+uboot and aardvark was initialized successfully.
> > > > > Without this patch series on ebin-17.10-uart.zip aardvark initialization
> > > > > failed.
> > > > > 
> > > > > Pali Rohár (2):
> > > > >   phy: marvell: comphy: Convert internal SMCC firmware return codes to
> > > > >     errno
> > > > >   PCI: aardvark: Fix initialization with old Marvell's Arm Trusted
> > > > >     Firmware
> > > > > 
> > > > >  drivers/pci/controller/pci-aardvark.c        |  4 +++-
> > > > >  drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 14 +++++++++++---
> > > > >  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 14 +++++++++++---
> > > > >  3 files changed, 25 insertions(+), 7 deletions(-)
> > > > 
> > > > Applied to pci/aardvark (both patches), thanks.
> > > 
> > > Ok! For second patch would be needed to put CC:stable line with
> > > specifying prerequisite of first patch as written in document:
> > > 
> > > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > > 
> > > If I understood it correctly, second patch needs following line:
> > > 
> > > Cc: <stable@vger.kernel.org> # <commit_id_of_first_path>: comphy: errno return codes
> > > 
> > > where <commit_id_of_first_path> is commit id of PATCH 1/2.
> > > 
> > > (correct me if I'm wrong)
> > > 
> > > Now when first commit has commit id, could you update second commit to
> > > include this information about prerequisite?
> > 
> > No I can't because they will be merged at the same time.
> 
> And it is a problem? Git commit id of first patch would not be changed,
> so referencing to it is now possible from second commit (unless you do
> rebasing).
> 
> > What we can do is mark the second patch for stable
> 
> This is done by adding "Cc: <stable@vger.kernel.org>" line to second
> patch which I suggested, right?

I will apply the stable tag and dependency, it should be fine.

Please ensure you follow up stable updates especially in case patches
don't backport properly (ie please try).

Lorenzo
Pali Rohár Oct. 2, 2020, 3:07 p.m. UTC | #9
On Friday 02 October 2020 16:03:09 Lorenzo Pieralisi wrote:
> On Fri, Oct 02, 2020 at 04:52:37PM +0200, Pali Rohár wrote:
> > On Friday 02 October 2020 15:38:51 Lorenzo Pieralisi wrote:
> > > On Fri, Oct 02, 2020 at 04:26:16PM +0200, Pali Rohár wrote:
> > > > On Friday 02 October 2020 14:37:13 Lorenzo Pieralisi wrote:
> > > > > On Wed, Sep 02, 2020 at 04:43:42PM +0200, Pali Rohár wrote:
> > > > > > This patch series fixes regression introduced in commit 366697018c9a
> > > > > > ("PCI: aardvark: Add PHY support") which caused aardvark driver
> > > > > > initialization failure on EspressoBin board with factory version of
> > > > > > Arm Trusted Firmware provided by Marvell.
> > > > > > 
> > > > > > Second patch depends on the first patch, so please add appropriate
> > > > > > Fixes/Cc:stable@ tags to have both patches correctly backported to
> > > > > > stable kernels.
> > > > > > 
> > > > > > I have tested both patches with Marvell ATF firmware ebin-17.10-uart.zip
> > > > > > and with upstream ATF+uboot and aardvark was initialized successfully.
> > > > > > Without this patch series on ebin-17.10-uart.zip aardvark initialization
> > > > > > failed.
> > > > > > 
> > > > > > Pali Rohár (2):
> > > > > >   phy: marvell: comphy: Convert internal SMCC firmware return codes to
> > > > > >     errno
> > > > > >   PCI: aardvark: Fix initialization with old Marvell's Arm Trusted
> > > > > >     Firmware
> > > > > > 
> > > > > >  drivers/pci/controller/pci-aardvark.c        |  4 +++-
> > > > > >  drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 14 +++++++++++---
> > > > > >  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 14 +++++++++++---
> > > > > >  3 files changed, 25 insertions(+), 7 deletions(-)
> > > > > 
> > > > > Applied to pci/aardvark (both patches), thanks.
> > > > 
> > > > Ok! For second patch would be needed to put CC:stable line with
> > > > specifying prerequisite of first patch as written in document:
> > > > 
> > > > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > > > 
> > > > If I understood it correctly, second patch needs following line:
> > > > 
> > > > Cc: <stable@vger.kernel.org> # <commit_id_of_first_path>: comphy: errno return codes
> > > > 
> > > > where <commit_id_of_first_path> is commit id of PATCH 1/2.
> > > > 
> > > > (correct me if I'm wrong)
> > > > 
> > > > Now when first commit has commit id, could you update second commit to
> > > > include this information about prerequisite?
> > > 
> > > No I can't because they will be merged at the same time.
> > 
> > And it is a problem? Git commit id of first patch would not be changed,
> > so referencing to it is now possible from second commit (unless you do
> > rebasing).
> > 
> > > What we can do is mark the second patch for stable
> > 
> > This is done by adding "Cc: <stable@vger.kernel.org>" line to second
> > patch which I suggested, right?
> 
> I will apply the stable tag and dependency, it should be fine.

Ok! I thought that according to stable-kernel-rules.html that dependent
commit could be added after stable email address separated with # char.
At least this is how I understood stable-kernel-rules.html and its
section:

  "Additionally, some patches submitted via Option 1 may have additional
   patch prerequisites which can be cherry-picked."

> Please ensure you follow up stable updates especially in case patches
> don't backport properly (ie please try).

No problem, I can monitor emails about backporting these patches.

> Lorenzo
Lorenzo Pieralisi Oct. 2, 2020, 3:15 p.m. UTC | #10
On Fri, Oct 02, 2020 at 05:07:01PM +0200, Pali Rohár wrote:

[...]

> > I will apply the stable tag and dependency, it should be fine.
> 
> Ok! I thought that according to stable-kernel-rules.html that dependent
> commit could be added after stable email address separated with # char.
> At least this is how I understood stable-kernel-rules.html and its
> section:
> 
>   "Additionally, some patches submitted via Option 1 may have additional
>    patch prerequisites which can be cherry-picked."

That's what I did - pci/aardvark branch.

Lorenzo
Pali Rohár Oct. 2, 2020, 3:20 p.m. UTC | #11
On Friday 02 October 2020 16:15:47 Lorenzo Pieralisi wrote:
> On Fri, Oct 02, 2020 at 05:07:01PM +0200, Pali Rohár wrote:
> 
> [...]
> 
> > > I will apply the stable tag and dependency, it should be fine.
> > 
> > Ok! I thought that according to stable-kernel-rules.html that dependent
> > commit could be added after stable email address separated with # char.
> > At least this is how I understood stable-kernel-rules.html and its
> > section:
> > 
> >   "Additionally, some patches submitted via Option 1 may have additional
> >    patch prerequisites which can be cherry-picked."
> 
> That's what I did - pci/aardvark branch.

Great, thank you!

I will be monitoring backport emails in case it would not be correctly
detected/cherrypicked.