Hi Niklas
On Mon, Feb 26, 2024 at 10:57:08AM +0100, Niklas Cassel wrote:
> Hello Andrey,
>
> On Sun, Feb 25, 2024 at 12:55:42PM +0300, Andrey Jr. Melnikov wrote:
> >
> > Stop using saved_port_map for masking port quirks, use force_port_map
> > instead.
> >
> > Signed-off-by: Andrey Jr. Melnikov <temnota.am@xxxxxxxxx>
> >
> > diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> > index 682ff550ccfb..066e3118801c 100644
> > --- a/drivers/ata/ahci.c
> > +++ b/drivers/ata/ahci.c
> > @@ -675,18 +675,18 @@ static void ahci_pci_save_initial_config(struct pci_dev *pdev,
> > switch (pdev->device) {
> > case 0x1166:
> > dev_info(&pdev->dev, "ASM1166 has only six ports\n");
> > - hpriv->saved_port_map = 0x3f;
> > + hpriv->mask_port_map = 0x3f;
> > break;
> > case 0x1064:
> > dev_info(&pdev->dev, "ASM1064 has only four ports\n");
> > - hpriv->saved_port_map = 0xf;
> > + hpriv->mask_port_map = 0xf;
> > break;
> > }
> > }
> >
> > if (pdev->vendor == PCI_VENDOR_ID_JMICRON && pdev->device == 0x2361) {
> > dev_info(&pdev->dev, "JMB361 has only one port\n");
> > - hpriv->saved_port_map = 1;
> > + hpriv->mask_port_map = 1;
> > }
> >
> > /*
> > diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> > index 1a63200ea437..cc705d3bdc50 100644
> > --- a/drivers/ata/libahci.c
> > +++ b/drivers/ata/libahci.c
> > @@ -531,16 +531,10 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)
> > cap &= ~HOST_CAP_SXS;
> > }
> >
> > - /* 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%lx -> 0x%x\n",
> > - port_map, hpriv->saved_port_map);
> > - port_map = hpriv->saved_port_map;
> > - } else {
> > - hpriv->saved_port_map = port_map;
> > - }
> > + hpriv->saved_port_map = port_map;
> >
> > + /* Override the HBA ports mapping if the platform needs it */
> > if (hpriv->mask_port_map) {
> > dev_warn(dev, "masking port_map 0x%lx -> 0x%lx\n",
> > port_map,
> >
>
> Before this patch, if there was a quirk, e.g. saved_port_map was set in
> ahci_pci_save_initial_config():
>
> then in ahci_save_initial_config(),
> we would not store/overwrite saved_port_map with readl(HOST_PORTS_IMPL).
>
> After this patch, saved_port_map will contain ports that might
> have been "disabled" by a quirk.
>
>
> Have you verified that this logical change is okay in all the
> places where saved_port_map is used?
>
> E.g.
> drivers/ata/ahci_dwc.c:ahci_dwc_check_cap() seems to iterate over:
> unsigned long port_map = hpriv->saved_port_map | hpriv->mask_port_map;
>
> which would be different before and after this patch.
>
> Serge, any comment?
>
>
>
> Also ahci_platform_get_firmware() seems to set
> saved_port_map based of device tree property "ports-implemented".
>
> Before this patch, saved_port map would still contain that value from
> device tree, after this patch, that saved_port_map will be overwritten
> with readl(HOST_PORTS_IMPL).
>
> Again, this code is authored by Serge. Serge, comments?
Thanks for Cc-ing me. Sorry, I am a bit busy right now. I'll have a
closer look on this patch later on this week.
-Serge(y)
>
>
> Kind regards,
> Niklas
