| Message ID | 20200911170738.82818-12-leobras.c@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | DDW Indirect Mapping | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (4b552a4cbf286ff9dcdab19153f3c1c7d1680fab) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 63 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
On 12/09/2020 03:07, Leonardo Bras wrote: > Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, > > Update remove_dma_window() so it can be used to remove DDW with a given > property name. > Out of context this seems useless. How about? === At the moment pseries stores information about created directly mapped DDW window in DIRECT64_PROPNAME. We are going to implement indirect DDW window which we need to preserve during kexec so we need another property for that. === Feel free to correct my english :) > This enables the creation of new property names for DDW, so we can > have different usage for it, like indirect mapping. > > Signed-off-by: Leonardo Bras <leobras.c@gmail.com> > --- > arch/powerpc/platforms/pseries/iommu.c | 21 +++++++++++---------- > 1 file changed, 11 insertions(+), 10 deletions(-) > > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c > index abd36b257725..f6a65ecd1db5 100644 > --- a/arch/powerpc/platforms/pseries/iommu.c > +++ b/arch/powerpc/platforms/pseries/iommu.c > @@ -818,31 +818,32 @@ static void remove_dma_window(struct device_node *np, u32 *ddw_avail, > np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN], liobn); > } > > -static void remove_ddw(struct device_node *np, bool remove_prop) > +static int remove_ddw(struct device_node *np, bool remove_prop, const char *win_name) > { > struct property *win; > u32 ddw_avail[DDW_APPLICABLE_SIZE]; > int ret = 0; > > + win = of_find_property(np, win_name, NULL); > + if (!win) > + return -EINVAL; > + > ret = of_property_read_u32_array(np, "ibm,ddw-applicable", > &ddw_avail[0], DDW_APPLICABLE_SIZE); > if (ret) > - return; > - > - win = of_find_property(np, DIRECT64_PROPNAME, NULL); > - if (!win) > - return; > + return 0; > > if (win->length >= sizeof(struct dynamic_dma_window_prop)) > remove_dma_window(np, ddw_avail, win); > > if (!remove_prop) > - return; > + return 0; > > ret = of_remove_property(np, win); > if (ret) > pr_warn("%pOF: failed to remove direct window property: %d\n", > np, ret); > + return 0; You do not test the return code anywhere until 13/14 so I'd say merge this one into 13/14, the same comment applies to 12/14. If you do not move chunks in 13/14, it is going to be fairly small patch. > } > > static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr) > @@ -894,7 +895,7 @@ static int find_existing_ddw_windows(void) > for_each_node_with_property(pdn, DIRECT64_PROPNAME) { > direct64 = of_get_property(pdn, DIRECT64_PROPNAME, &len); > if (!direct64 || len < sizeof(*direct64)) { > - remove_ddw(pdn, true); > + remove_ddw(pdn, true, DIRECT64_PROPNAME); > continue; > } > > @@ -1325,7 +1326,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn) > win64 = NULL; > > out_win_del: > - remove_ddw(pdn, true); > + remove_ddw(pdn, true, DIRECT64_PROPNAME); > > out_failed: > if (default_win_removed) > @@ -1480,7 +1481,7 @@ static int iommu_reconfig_notifier(struct notifier_block *nb, unsigned long acti > * we have to remove the property when releasing > * the device node. > */ > - remove_ddw(np, false); > + remove_ddw(np, false, DIRECT64_PROPNAME); > if (pci && pci->table_group) > iommu_pseries_free_group(pci->table_group, > np->full_name); >
On Tue, 2020-09-29 at 13:56 +1000, Alexey Kardashevskiy wrote: > > On 12/09/2020 03:07, Leonardo Bras wrote: > > Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, > > > > Update remove_dma_window() so it can be used to remove DDW with a given > > property name. > > > > Out of context this seems useless. How about? > === > At the moment pseries stores information about created directly mapped > DDW window in DIRECT64_PROPNAME. We are going to implement indirect DDW > window which we need to preserve during kexec so we need another > property for that. > === > > Feel free to correct my english :) Thanks Alexey! It helped a lot me better describing the reasoning before the change! > > > > ret = of_remove_property(np, win); > > if (ret) > > pr_warn("%pOF: failed to remove direct window property: %d\n", > > np, ret); > > + return 0; > > > You do not test the return code anywhere until 13/14 so I'd say merge > this one into 13/14, the same comment applies to 12/14. If you do not > move chunks in 13/14, it is going to be fairly small patch. I have applied most suggested changes for patches 11,12,13, but on a single diff it still amounts to 275 lines. To be honest, after 7 months of sending this patchset (and working on other stuff), patch 13 looks a lot like to read alone, and merging with 11 & 12 seems to be too much. Would it be ok to apply the changes and leave them all separated, or as a mid ground just merging 11 & 12 together? Adding your suggested text above should be enough to get enough context for them. I could also say why the return code is left unused for now. Best regards, Leonardo Bras
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c index abd36b257725..f6a65ecd1db5 100644 --- a/arch/powerpc/platforms/pseries/iommu.c +++ b/arch/powerpc/platforms/pseries/iommu.c @@ -818,31 +818,32 @@ static void remove_dma_window(struct device_node *np, u32 *ddw_avail, np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN], liobn); } -static void remove_ddw(struct device_node *np, bool remove_prop) +static int remove_ddw(struct device_node *np, bool remove_prop, const char *win_name) { struct property *win; u32 ddw_avail[DDW_APPLICABLE_SIZE]; int ret = 0; + win = of_find_property(np, win_name, NULL); + if (!win) + return -EINVAL; + ret = of_property_read_u32_array(np, "ibm,ddw-applicable", &ddw_avail[0], DDW_APPLICABLE_SIZE); if (ret) - return; - - win = of_find_property(np, DIRECT64_PROPNAME, NULL); - if (!win) - return; + return 0; if (win->length >= sizeof(struct dynamic_dma_window_prop)) remove_dma_window(np, ddw_avail, win); if (!remove_prop) - return; + return 0; ret = of_remove_property(np, win); if (ret) pr_warn("%pOF: failed to remove direct window property: %d\n", np, ret); + return 0; } static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr) @@ -894,7 +895,7 @@ static int find_existing_ddw_windows(void) for_each_node_with_property(pdn, DIRECT64_PROPNAME) { direct64 = of_get_property(pdn, DIRECT64_PROPNAME, &len); if (!direct64 || len < sizeof(*direct64)) { - remove_ddw(pdn, true); + remove_ddw(pdn, true, DIRECT64_PROPNAME); continue; } @@ -1325,7 +1326,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn) win64 = NULL; out_win_del: - remove_ddw(pdn, true); + remove_ddw(pdn, true, DIRECT64_PROPNAME); out_failed: if (default_win_removed) @@ -1480,7 +1481,7 @@ static int iommu_reconfig_notifier(struct notifier_block *nb, unsigned long acti * we have to remove the property when releasing * the device node. */ - remove_ddw(np, false); + remove_ddw(np, false, DIRECT64_PROPNAME); if (pci && pci->table_group) iommu_pseries_free_group(pci->table_group, np->full_name);
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, Update remove_dma_window() so it can be used to remove DDW with a given property name. This enables the creation of new property names for DDW, so we can have different usage for it, like indirect mapping. Signed-off-by: Leonardo Bras <leobras.c@gmail.com> --- arch/powerpc/platforms/pseries/iommu.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)