linux-kernel - Re: [PATCH 2/2] PCI: meson: add the Amlogic Meson PCIe phy driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e5d9d89e79f8ec37b6337e54912f49a0b0856874.camel@baylibre.com>
Date:   Thu, 16 Aug 2018 10:33:39 +0200
From:   Jerome Brunet <jbrunet@...libre.com>
To:     Hanjie Lin <hanjie.lin@...ogic.com>,
        Kishon Vijay Abraham I <kishon@...com>
Cc:     Yue Wang <yue.wang@...ogic.com>, linux-kernel@...r.kernel.org,
        linux-amlogic@...ts.infradead.org, linux-pci@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        Kevin Hilman <khilman@...libre.com>,
        Carlo Caione <carlo@...one.org>, Rob Herring <robh@...nel.org>,
        shawn.lin@...k-chips.com
Subject: Re: [PATCH 2/2] PCI: meson: add the Amlogic Meson PCIe phy driver

On Thu, 2018-08-16 at 11:05 +0800, Hanjie Lin wrote:
> 
> On 2018/8/14 18:41, Jerome Brunet wrote:
> > On Tue, 2018-08-14 at 02:12 -0400, Hanjie Lin wrote:
> > > From: Yue Wang <yue.wang@...ogic.com>
> > > 
> > > The Meson-PCIE-PHY controller supports the 5-Gbps data rate
> > > of the PCI Express Gen 2 specification and is backwardcompatible
> > > with the 2.5-Gbps Gen 1.1 specification with only
> > > inferred idle detection supported on AMLOGIC SoCs.
> > 
> > It looks like the sole purpose of this driver is to provide the reset lines to
> > pcie driver.
> > 
> > I wonder why we need this ? Can't the pcie driver claim the reset lines itself.
> > 
> > Also, an init of this phy will always reset both port. What will happen if the
> > first port is in use and the 2nd port comes up ?? 
> > 
> > Looks the the pcie driver should claim 'apb' and 'phy' reset lines as "shared"
> > reset and the required 'port' as 'exclusive'
> > 
> 
> Thank you for your response.
> 
> Yes, 'apb' and 'phy' reset lines are shared, and ‘port' reset line is exclusive.
> If we handle all reset lines during the first port initial sequence, 
> and when the second port comes up, we will do nothing about the rest lines, 
> and don't need a extra API to do ‘port' reset;

With which other driver are your control shared ?

Looks it is the answer is none since this phy driver will reset both port
already, even if one is used.

In this case the fact that you are using shared control is just abusing the
framework to reset once.

As far as I can tell, this driver makes no sense. The appropriate reset lines
should be given directly to your pcie driver.