From: Serge Semin <fancer.lancer@gmail.com>
To: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>,
Hans de Goede <hdegoede@redhat.com>, Jens Axboe <axboe@kernel.dk>,
Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>,
Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>,
Rob Herring <robh+dt@kernel.org>,
linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org
Subject: Re: [PATCH 12/21] ata: libahci: Discard redundant force_port_map parameter
Date: Mon, 11 Apr 2022 15:11:51 +0300 [thread overview]
Message-ID: <20220411121151.vm6wmtalbl2lgtgo@mobilestation> (raw)
In-Reply-To: <b06a8382-d5c1-e3a5-8577-692fa82cb3c1@opensource.wdc.com>
On Thu, Mar 24, 2022 at 11:05:58AM +0900, Damien Le Moal wrote:
> On 3/24/22 09:16, Serge Semin wrote:
> > Currently there are four port-map-related fields declared in the
> > ahci_host_priv structure and used to setup the HBA ports mapping. First
> > the ports-mapping is read from the PI register and immediately stored in
> > the saved_port_map field. If forced_port_map is initialized with non-zero
> > value then its value will have greater priority over the value read from
> > PI, thus it will override the saved_port_map field. That value will be then
> > masked by a non-zero mask_port_map field and after some sanity checks it
> > will be stored in the ahci_host_priv.port_map field as a final port
> > mapping.
> >
> > As you can see the logic is a bit too complicated for such a simple task.
> > We can freely get rid from at least one of the fields with no change to
> > the implemented semantic. The force_port_map field can be replaced with
> > taking non-zero saved_port_map value into account. So if saved_port_map is
> > pre-initialized by the glue-driver/platform-specific code then it will
>
> glue-driver == LLDD (low level device driver), for the entire series please.
Why? It's a normal term and well known amongst developers. I've used it
in a plenty of my patches before and none of them has been questioned in that
part so far.
-Sergey
>
> > have greater priority over the value read from PI register and will be
> > used as actual HBA ports mapping later on. Thus the ports map forcing task
> > will be just transferred from the force_port_map to saved_port_map field.
> >
> > This modification will perfectly fit into the feature of having OF-based
> > initialization of the HW-init HBA CSR fields we are about to introduce in
> > the next commit.
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > ---
> > drivers/ata/ahci.c | 2 +-
> > drivers/ata/ahci.h | 1 -
> > drivers/ata/libahci.c | 10 ++++++----
> > drivers/ata/libahci_platform.c | 2 +-
> > 4 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> > index ab5811ef5a53..8ce0d166cc8d 100644
> > --- a/drivers/ata/ahci.c
> > +++ b/drivers/ata/ahci.c
> > @@ -654,7 +654,7 @@ static void ahci_pci_save_initial_config(struct pci_dev *pdev,
> > {
> > if (pdev->vendor == PCI_VENDOR_ID_JMICRON && pdev->device == 0x2361) {
> > dev_info(&pdev->dev, "JMB361 has only one port\n");
> > - hpriv->force_port_map = 1;
> > + hpriv->saved_port_map = 1;
> > }
> >
> > /*
> > diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> > index 04690b4168a3..519d148ecaea 100644
> > --- a/drivers/ata/ahci.h
> > +++ b/drivers/ata/ahci.h
> > @@ -329,7 +329,6 @@ struct ahci_port_priv {
> > struct ahci_host_priv {
> > /* Input fields */
> > unsigned int flags; /* AHCI_HFLAG_* */
> > - u32 force_port_map; /* force port map */
> > u32 mask_port_map; /* mask out particular bits */
> >
> > void __iomem * mmio; /* bus-independent mem map */
> > diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> > index 0ed484e04fd6..011175e82174 100644
> > --- a/drivers/ata/libahci.c
> > +++ b/drivers/ata/libahci.c
> > @@ -453,7 +453,6 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)
> > * reset. Values without are used for driver operation.
> > */
> > hpriv->saved_cap = cap = readl(mmio + HOST_CAP);
> > - hpriv->saved_port_map = port_map = readl(mmio + HOST_PORTS_IMPL);
> >
> > /* CAP2 register is only defined for AHCI 1.2 and later */
> > vers = readl(mmio + HOST_VERSION);
> > @@ -517,10 +516,13 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)
> > cap &= ~HOST_CAP_SXS;
> > }
> >
> > - if (hpriv->force_port_map && port_map != hpriv->force_port_map) {
> > + /* Override the HBA ports mapping if the platform needs it */
> > + port_map = readl(mmio + HOST_PORTS_IMPL);
> > + if (hpriv->saved_port_map && port_map != hpriv->saved_port_map) {
> > dev_info(dev, "forcing port_map 0x%x -> 0x%x\n",
> > - port_map, hpriv->force_port_map);
> > - port_map = hpriv->force_port_map;
> > + port_map, hpriv->saved_port_map);
> > + port_map = hpriv->saved_port_map;
> > + } else {
> > hpriv->saved_port_map = port_map;
> > }
> >
> > diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> > index febad33aa43c..5cbc2c42164d 100644
> > --- a/drivers/ata/libahci_platform.c
> > +++ b/drivers/ata/libahci_platform.c
> > @@ -539,7 +539,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
> > }
> >
> > of_property_read_u32(dev->of_node,
> > - "ports-implemented", &hpriv->force_port_map);
> > + "ports-implemented", &hpriv->saved_port_map);
> >
> > if (child_nodes) {
> > for_each_child_of_node(dev->of_node, child) {
>
>
> --
> Damien Le Moal
> Western Digital Research
next prev parent reply other threads:[~2022-04-11 12:12 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-24 0:16 [PATCH 00/21] ata: ahci: Add DWC/Baikal-T1 AHCI SATA support Serge Semin
2022-03-24 0:16 ` [PATCH 01/21] dt-bindings: ata: sata: Extend number of SATA ports Serge Semin
2022-03-29 8:15 ` Damien Le Moal
2022-04-11 19:25 ` Serge Semin
2022-03-24 0:16 ` [PATCH 02/21] dt-bindings: ata: Convert AHCI-bindings to DT schema Serge Semin
2022-03-28 19:32 ` Rob Herring
2022-04-11 19:34 ` Serge Semin
2022-03-24 0:16 ` [PATCH 03/21] ata: libahci_platform: Explicitly set rc on devres_alloc failure Serge Semin
2022-03-24 0:58 ` Damien Le Moal
2022-03-24 21:37 ` Serge Semin
2022-03-25 1:56 ` Damien Le Moal
2022-04-06 20:03 ` Serge Semin
2022-03-29 8:20 ` Damien Le Moal
2022-03-24 0:16 ` [PATCH 04/21] ata: libahci_platform: Convert to using handy devm-ioremap methods Serge Semin
2022-03-24 1:11 ` Damien Le Moal
2022-04-06 20:42 ` Serge Semin
2022-03-24 0:16 ` [PATCH 05/21] ata: libahci_platform: Convert to using devm bulk clocks API Serge Semin
2022-03-24 1:29 ` Damien Le Moal
2022-04-09 4:59 ` Serge Semin
2022-03-28 22:36 ` kernel test robot
2022-03-28 23:42 ` kernel test robot
2022-03-29 0:03 ` kernel test robot
2022-03-24 0:16 ` [PATCH 06/21] ata: libahci_platform: Add function returning a clock-handle by id Serge Semin
2022-03-24 1:36 ` Damien Le Moal
2022-04-11 6:02 ` Serge Semin
2022-03-24 0:16 ` [PATCH 07/21] ata: libahci_platform: Sanity check the DT child nodes number Serge Semin
2022-03-24 1:40 ` Damien Le Moal
2022-03-24 8:12 ` Sergey Shtylyov
2022-03-24 8:13 ` Damien Le Moal
2022-04-11 13:10 ` Serge Semin
2022-03-24 0:16 ` [PATCH 08/21] ata: libahci_platform: Parse ports-implemented property in resources getter Serge Semin
2022-03-24 0:16 ` [PATCH 09/21] ata: libahci_platform: Introduce reset assertion/deassertion methods Serge Semin
2022-03-24 1:52 ` Damien Le Moal
2022-04-11 10:57 ` Serge Semin
2022-03-24 0:16 ` [PATCH 10/21] dt-bindings: ata: ahci: Add platform capability properties Serge Semin
2022-03-24 0:16 ` [PATCH 11/21] ata: libahci: Extend port-cmd flags set with port capabilities Serge Semin
2022-03-24 0:16 ` [PATCH 12/21] ata: libahci: Discard redundant force_port_map parameter Serge Semin
2022-03-24 2:05 ` Damien Le Moal
2022-04-11 12:11 ` Serge Semin [this message]
2022-04-11 12:25 ` Damien Le Moal
2022-04-11 20:52 ` Serge Semin
2022-04-11 22:48 ` Damien Le Moal
2022-04-12 12:29 ` Serge Semin
2022-03-24 0:16 ` [PATCH 13/21] ata: libahci: Don't read AHCI version twice in the save-config method Serge Semin
2022-03-24 0:16 ` [PATCH 14/21] ata: ahci: Convert __ahci_port_base to accepting hpriv as arguments Serge Semin
2022-03-24 0:16 ` [PATCH 15/21] ata: ahci: Introduce firmware-specific caps initialization Serge Semin
2022-03-24 0:16 ` [PATCH 16/21] dt-bindings: ata: ahci: Add DWC AHCI SATA controller DT schema Serge Semin
2022-04-01 0:06 ` Rob Herring
2022-04-11 20:00 ` Serge Semin
2022-03-24 0:16 ` [PATCH 17/21] ata: ahci: Add DWC AHCI SATA controller support Serge Semin
2022-03-24 2:21 ` Damien Le Moal
2022-04-11 12:41 ` Serge Semin
2022-04-11 13:03 ` Damien Le Moal
2022-04-11 20:41 ` Serge Semin
2022-03-24 0:16 ` [PATCH 18/21] dt-bindings: ata: ahci: Add Baikal-T1 AHCI SATA controller DT schema Serge Semin
2022-03-24 0:16 ` [PATCH 19/21] ata: ahci-dwc: Add platform-specific quirks support Serge Semin
2022-03-24 0:16 ` [PATCH 20/21] ata: ahci-dwc: Add Baikal-T1 AHCI SATA interface support Serge Semin
2022-03-24 0:16 ` [PATCH 21/21] MAINTAINERS: Add maintainers for DWC AHCI SATA driver Serge Semin
2022-03-24 2:17 ` Damien Le Moal
2022-04-11 12:16 ` Serge Semin
2022-03-28 20:06 ` [PATCH 00/21] ata: ahci: Add DWC/Baikal-T1 AHCI SATA support Damien Le Moal
2022-03-29 8:30 ` Damien Le Moal
2022-04-06 19:54 ` Serge Semin
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=20220411121151.vm6wmtalbl2lgtgo@mobilestation \
--to=fancer.lancer@gmail.com \
--cc=Alexey.Malahov@baikalelectronics.ru \
--cc=Pavel.Parkhomenko@baikalelectronics.ru \
--cc=Sergey.Semin@baikalelectronics.ru \
--cc=axboe@kernel.dk \
--cc=damien.lemoal@opensource.wdc.com \
--cc=devicetree@vger.kernel.org \
--cc=hdegoede@redhat.com \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robh+dt@kernel.org \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).