linux-kernel - RE: [PATCH] PCI: xilinx-nwl: Remove unnecessary code and updating ecam default value.

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] [day] [month] [year] [list]

Message-ID: <SN7PR12MB72018BE0D2DD36ED2B86C2D08B0EA@SN7PR12MB7201.namprd12.prod.outlook.com>
Date:   Sat, 5 Aug 2023 19:07:50 +0000
From:   "Havalige, Thippeswamy" <thippeswamy.havalige@....com>
To:     Bjorn Helgaas <helgaas@...nel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "krzysztof.kozlowski@...aro.org" <krzysztof.kozlowski@...aro.org>,
        "lpieralisi@...nel.org" <lpieralisi@...nel.org>,
        "Gogada, Bharat Kumar" <bharat.kumar.gogada@....com>,
        "Simek, Michal" <michal.simek@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH] PCI: xilinx-nwl: Remove unnecessary code and updating
 ecam default value.

Hi Bjorn,

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@...nel.org>
> Sent: Saturday, August 5, 2023 1:28 AM
> To: Havalige, Thippeswamy <thippeswamy.havalige@....com>
> Cc: linux-kernel@...r.kernel.org; robh+dt@...nel.org;
> bhelgaas@...gle.com; linux-pci@...r.kernel.org;
> krzysztof.kozlowski@...aro.org; lpieralisi@...nel.org; Gogada, Bharat Kumar
> <bharat.kumar.gogada@....com>; Simek, Michal
> <michal.simek@....com>; linux-arm-kernel@...ts.infradead.org
> Subject: Re: [PATCH] PCI: xilinx-nwl: Remove unnecessary code and updating
> ecam default value.
> 
> On Fri, Aug 04, 2023 at 07:05:30PM +0000, Havalige, Thippeswamy wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@...nel.org>
> > > On Thu, Aug 03, 2023 at 05:20:16PM +0530, Thippeswamy Havalige wrote:
> > > > Remove reduntant code.
> > > > Change NWL_ECAM_VALUE_DEFAULT to 16 to support maximum 256
> > > > buses.
> > >
> > > Remove period from subject line.
> > >
> > > Please mention the most important part first in the subject -- the
> > > ECAM change sounds more important than removing redundant code.
> > >
> > > s/ecam/ECAM/
> > > s/reduntant/redundant/
> > >
> > > Please elaborate on why this code is redundant.  What makes it
> > > redundant?  Apparently the bus number registers default to the correct
> > > values or some other software programs them?
> >
> >  - Yes, The  Primary,Secondary and sub-ordinate bus number registers
> >  are programmed/updated as part of linux enumeration so updating
> >  these registers are redundant.
> 
> Ah, so the Linux PCI core can handle updating these from whatever the
> power-up values are.  Good material for the revised commit log.
> 
> > > "ECAM_VALUE" is not a very informative name.  I don't know what an
> > > "ECAM value" would be.  How is the value 16 related to a maximum of
> > > 256 buses?  We only need 8 bits to address 256 buses, so it must be
> > > something else.  The bus number appears at bits 20-27
> > > (PCIE_ECAM_BUS_SHIFT) in a standard ECAM address, so probably not
> the
> > > bit location?
> >
> > Yes, Agreed I'll modify ECAM_VALUE as ECAM_SIZE here and it is not
> > related to a maximum 256 buses.
> 
> Well, it sounds like this value *does* determine the size of the ECAM
> region, which does constrain the number of buses you can address via
> ECAM.
> 
- Yes, This ecam_size does determine the number of buses can be addressed via ECAM.
> > > Does this fix a problem?
> >
> > - Yes, It is fixing a problem. Our controller is expecting ECAM size
> > to be programmed by software.  By programming
> > "NWL_ECAM_VALUE_DEFAULT  12" controller can access upto 16MB ECAM
> > region which is used to detect 16 buses so by updating
> > "NWL_ECAM_VALUE_DEFAULT " to 16 controller can access upto 256 Mb
> > ECAM region to detect 256 buses.
> >
> > 2^(ecam_size_offset+ecam_size)
> >
> > Here (ecam_size_offset=12 and ecam_size=16) --> 256Mb
> 
> More good material for the commit log.  (1) Change in ECAM region
> size, (2) previously could only address 16 buses, now can address 256
> buses.
> 
> Is there any impact on DT from the address map change?
> 
- Yes. Now device tree ECAM size needs to be modified to 256Mb.
- I'll update device tree changes along with next patch.
> Bjorn

Regards,
Thippeswamy H