Messages in this thread | | | From | Andy Shevchenko <> | Date | Mon, 6 Mar 2017 14:20:24 +0200 | Subject | Re: [PATCH 1/5] PCI: add resizeable BAR infrastructure v2 |
| |
On Mon, Mar 6, 2017 at 1:40 PM, Christian König <deathsimple@vodafone.de> wrote: > From: Christian König <christian.koenig@amd.com> > > Just the defines and helper functions to read the possible sizes of a BAR and > update it's size. > > See https://pcisig.com/sites/default/files/specification_documents/ECN_Resizable-BAR_24Apr2008.pdf. > > v2: provide read helper as well
Commit message left away the explanation at which point this API might be useful and how it fits in managed resources model?
> /** > + * pci_rbar_get_sizes - get possible sizes for BAR
Why not simple pci_rbar_get_possible_sizes() ?
> +u32 pci_rbar_get_sizes(struct pci_dev *pdev, int bar) > +{ > + int pos, nbars; > + u32 ctrl, cap; > + int i; > + > + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR); > + if (!pos)
> + return 0x0;
return 0;
> + > + pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl); > + nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >> PCI_REBAR_CTRL_NBAR_SHIFT; > +
> + for (i = 0; i < nbars; ++i, pos += 8) {
8 is defined somewhere in the spec? (Yes, I understand that is just 64 bits shift)
> + int bar_idx; > + > + pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl); > + bar_idx = (ctrl & PCI_REBAR_CTRL_BAR_IDX_MASK) >> > + PCI_REBAR_CTRL_BAR_IDX_SHIFT; > + if (bar_idx != bar) > + continue; > + > + pci_read_config_dword(pdev, pos + PCI_REBAR_CAP, &cap); > + return (cap & PCI_REBAR_CTRL_SIZES_MASK) >> > + PCI_REBAR_CTRL_SIZES_SHIFT; > + } > +
> + return 0x0;
return 0;
> +/** > + * pci_rbar_get_size - get the current size of a BAR
pci_rbar_get_current_size() ?
> +/** > + * pci_rbar_set_size - set a new size for a BAR > + * @dev: PCI device > + * @bar: BAR to set size to
> + * @size: new size as defined in the spec.
* @size: bitmasked value of new size (bit 0=1MB, ..., bit 19=512G)
?
It will briefly get a clue without reading either spec or long description.
> + * > + * Set the new size of a BAR as defined in the spec (0=1MB, 19=512GB). > + * Returns true if resizing was successful, false otherwise. > + */
> +bool pci_rbar_set_size(struct pci_dev *pdev, int bar, int size)
I would return int and error code. It would be better in the future and seems in alignment with above.
> +{ > + int pos, nbars; > + u32 ctrl; > + int i;
All ints are unsigned?
-- With Best Regards, Andy Shevchenko
| |