> -----Original Message-----
> From: [email protected] [mailto:linux-gpio-
> [email protected]] On Behalf Of Andy Shevchenko
> Sent: Friday, November 11, 2016 12:07 AM
> To: Tan, Jui Nee <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; Yong, Jonathan <[email protected]>;
> Yu, Ong Hock <[email protected]>; Luck, Tony <[email protected]>;
> Wan Mohamad, Wan Ahmad Zainie <[email protected]>;
> Sun, Yunying <[email protected]>
> Subject: Re: [PATCH v10 1/6] drivers/platform/x86/p2sb: New Primary to
> Sideband bridge support driver for Intel SOC's
> 
> On Thu, 2016-11-10 at 17:00 +0800, Tan Jui Nee wrote:
> > From: Andy Shevchenko <[email protected]>
> >
> > There is already one and at least one more user coming which require
> > an access to Primary to Sideband bridge (P2SB) in order to get IO or
> > MMIO bar hidden by BIOS.
> > Create a driver to access P2SB for x86 devices.
> >
> > Signed-off-by: Yong, Jonathan <[email protected]>
> > Signed-off-by: Andy Shevchenko <[email protected]>
> 
> 
> > +int p2sb_bar(struct pci_dev *pdev, unsigned int devfn,
> > +   struct resource *res)
> > +{
> > +   u32 base_addr;
> > +   u64 base64_addr;
> > +   unsigned long flags;
> > +
> >
> 
> > +   if (!res)
> > +           return -EINVAL;
> 
> I don't remember the details, one version was quite changed, so, I think
> these lines are not needed anymore.
> 
Noted, these lines will be removed in next patch version (v12).
> > +   /* Get IO or MMIO BAR */
> > +   pci_bus_read_config_dword(pdev->bus, devfn, SBREG_BAR,
> > &base_addr);
> > +   if ((base_addr & PCI_BASE_ADDRESS_SPACE) ==
> > PCI_BASE_ADDRESS_SPACE_IO) {
> > +           flags = IORESOURCE_IO;
> > +           base64_addr = base_addr & PCI_BASE_ADDRESS_IO_MASK;
> > +   } else {
> > +           flags = IORESOURCE_MEM;
> > +           base64_addr = base_addr & PCI_BASE_ADDRESS_MEM_MASK;
> > +           if (base_addr & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> > +                   flags |= IORESOURCE_MEM_64;
> >
> 
> > +                   pci_bus_read_config_dword(pdev->bus, devfn,
> > +                           SBREG_BAR + 4, &base_addr);
> 
> Fix indentation.
> 
Thanks for pointing that out. I will fix that in next patch version (v12). 
> > +                   base64_addr |= (u64)base_addr << 32;
> > +           }
> > +   }
> > +
> > +   /* Hide the P2SB device */
> > +   pci_bus_write_config_byte(pdev->bus, devfn, SBREG_HIDE,
> > 0x01);
> > +
> > +   spin_unlock(&p2sb_spinlock);
> > +
> 
> > +   /* User provides prefilled resources */
> 
> Not anymore as far I as I can see. You just return here the result.
> 
Current version is returning status of p2sb_bar function, i.e., 0 on success or 
appropriate errno value on error. Perhaps you could share the reason of return 
the result instead of status. 
> > +   res->start = (resource_size_t)base64_addr;
> > +   res->flags = flags;
> 
> --
> Andy Shevchenko <[email protected]>
> Intel Finland Oy
> --
> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
> the body of a message to [email protected] More majordomo info at
> http://vger.kernel.org/majordomo-info.html

Reply via email to