Message ID | 20220324001628.13028-13-Sergey.Semin@baikalelectronics.ru |
---|---|
State | New |
Headers | show |
Series | ata: ahci: Add DWC/Baikal-T1 AHCI SATA support | expand |
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
On 4/11/22 21:11, Serge Semin wrote: > 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. This term is not used in libata, nor do I remember seeing it used in SCSI or block subsystem either. We always talk about mid-layer (ahci platform) and LLDD (adapter driver). > > -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
On Mon, Apr 11, 2022 at 09:25:03PM +0900, Damien Le Moal wrote: > On 4/11/22 21:11, Serge Semin wrote: > > 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. > > This term is not used in libata, nor do I remember seeing it used in SCSI > or block subsystem either. We always talk about mid-layer (ahci platform) > and LLDD (adapter driver). Great, do we need to learn the subsystem-specific dictionary now before submitting the patches for it? =) Really, you are asking me to change one term to its synonym just because it's mainly unused here. Now you know what it means, the others can easily google it and get to learn something new. Win-win.) -Sergey > > > > > -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 > > > -- > Damien Le Moal > Western Digital Research
On 4/12/22 05:52, Serge Semin wrote: > On Mon, Apr 11, 2022 at 09:25:03PM +0900, Damien Le Moal wrote: >> On 4/11/22 21:11, Serge Semin wrote: >>> 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. >> > >> This term is not used in libata, nor do I remember seeing it used in SCSI >> or block subsystem either. We always talk about mid-layer (ahci platform) >> and LLDD (adapter driver). > > Great, do we need to learn the subsystem-specific dictionary now > before submitting the patches for it? =) > Really, you are asking me to change one term to its synonym just > because it's mainly unused here. Now you know what it means, the > others can easily google it and get to learn something new. Win-win.) I already knew what it meant, but it was unclear if my idea of what you meant was actually the same as yours. Sticking with the vocabulary already used since *a long time ago* makes life easier for reviewers and avoids confusion.
On Tue, Apr 12, 2022 at 07:48:50AM +0900, Damien Le Moal wrote: > On 4/12/22 05:52, Serge Semin wrote: > > On Mon, Apr 11, 2022 at 09:25:03PM +0900, Damien Le Moal wrote: > >> On 4/11/22 21:11, Serge Semin wrote: > >>> 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. > >> > > > >> This term is not used in libata, nor do I remember seeing it used in SCSI > >> or block subsystem either. We always talk about mid-layer (ahci platform) > >> and LLDD (adapter driver). > > > > Great, do we need to learn the subsystem-specific dictionary now > > before submitting the patches for it? =) > > Really, you are asking me to change one term to its synonym just > > because it's mainly unused here. Now you know what it means, the > > others can easily google it and get to learn something new. Win-win.) > > I already knew what it meant, but it was unclear if my idea of what you > meant was actually the same as yours. Sticking with the vocabulary already > used since *a long time ago* makes life easier for reviewers and avoids > confusion. I believe there can't be too many possible meaning of that term to have much doubts. Especially when we refer to the kernel drivers. One more time requesting to settle some implicit subsystem-specific conventions, not having them described in some kernel documents seems very much wrong. The ahci_ prefixing and the specific vocabulary concerns each driver in very many aspects. Seeing there are not a few drivers which don't follow those conventions, you give no chance for the developers to get their code accepted with no requests to fix the corresponding parts. So to speak you need to thoroughly describe these and the others conventions in the kernel documentation otherwise you'll always end up requesting the same fixes wasting your and developers time again and again. Really if you had that document available, you could have used as a reference and just said something like "please, follow the coding style convention described here..." and no question would have raised. Meanwhile currently both ahci_-prefix change and using the LLDD term seem more like a personal desire to me. -Sergey > > -- > Damien Le Moal > Western Digital Research
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) {
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 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(-)