| Message ID | 165853776328.2430596.4647259305040072751.stgit@dwillia2-xfh.jf.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | CXL Region Provisioning Fixes | expand |
On Fri, Jul 22, 2022 at 05:56:03PM -0700, Dan Williams wrote: > For switch and endpoint decoders the relationship of decoders to regions > is 1:1. However, for root decoders the relationship is 1:N. Also, > regions are already children of root decoders, so the 1:N relationship > is observed by walking the following glob: > > /sys/bus/cxl/devices/$decoder/region* > > Hide the vestigial 'region' attribute for root decoders. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Alison Schofield <alison.schofield@intel.com> > --- > drivers/cxl/core/port.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index 3d2d0119cc3d..bffde862de0b 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -290,7 +290,6 @@ static struct attribute *cxl_decoder_base_attrs[] = { > &dev_attr_locked.attr, > &dev_attr_interleave_granularity.attr, > &dev_attr_interleave_ways.attr, > - SET_CXL_REGION_ATTR(region) > NULL, > }; > > @@ -345,6 +344,7 @@ static const struct attribute_group *cxl_decoder_root_attribute_groups[] = { > static struct attribute *cxl_decoder_switch_attrs[] = { > &dev_attr_target_type.attr, > &dev_attr_target_list.attr, > + SET_CXL_REGION_ATTR(region) > NULL, > }; > > @@ -364,6 +364,7 @@ static struct attribute *cxl_decoder_endpoint_attrs[] = { > &dev_attr_mode.attr, > &dev_attr_dpa_size.attr, > &dev_attr_dpa_resource.attr, > + SET_CXL_REGION_ATTR(region) > NULL, > }; > >
On Fri, 2022-07-22 at 17:56 -0700, Dan Williams wrote: > For switch and endpoint decoders the relationship of decoders to > regions > is 1:1. However, for root decoders the relationship is 1:N. Also, > regions are already children of root decoders, so the 1:N > relationship > is observed by walking the following glob: > > /sys/bus/cxl/devices/$decoder/region* > > Hide the vestigial 'region' attribute for root decoders. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/cxl/core/port.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index 3d2d0119cc3d..bffde862de0b 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -290,7 +290,6 @@ static struct attribute *cxl_decoder_base_attrs[] > = { > &dev_attr_locked.attr, > &dev_attr_interleave_granularity.attr, > &dev_attr_interleave_ways.attr, > - SET_CXL_REGION_ATTR(region) > NULL, > }; > > @@ -345,6 +344,7 @@ static const struct attribute_group > *cxl_decoder_root_attribute_groups[] = { > static struct attribute *cxl_decoder_switch_attrs[] = { > &dev_attr_target_type.attr, > &dev_attr_target_list.attr, > + SET_CXL_REGION_ATTR(region) This patch removes the 'region' attr from switch decoders, but isn't it also vestigial under root decoders? > NULL, > }; > > @@ -364,6 +364,7 @@ static struct attribute > *cxl_decoder_endpoint_attrs[] = { > &dev_attr_mode.attr, > &dev_attr_dpa_size.attr, > &dev_attr_dpa_resource.attr, > + SET_CXL_REGION_ATTR(region) > NULL, > }; > >
On Mon, 2022-08-01 at 19:38 +0000, Verma, Vishal L wrote: > On Fri, 2022-07-22 at 17:56 -0700, Dan Williams wrote: > > For switch and endpoint decoders the relationship of decoders to > > regions > > is 1:1. However, for root decoders the relationship is 1:N. Also, > > regions are already children of root decoders, so the 1:N > > relationship > > is observed by walking the following glob: > > > > /sys/bus/cxl/devices/$decoder/region* > > > > Hide the vestigial 'region' attribute for root decoders. > > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > --- > > drivers/cxl/core/port.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > > index 3d2d0119cc3d..bffde862de0b 100644 > > --- a/drivers/cxl/core/port.c > > +++ b/drivers/cxl/core/port.c > > @@ -290,7 +290,6 @@ static struct attribute *cxl_decoder_base_attrs[] > > = { > > &dev_attr_locked.attr, > > &dev_attr_interleave_granularity.attr, > > &dev_attr_interleave_ways.attr, > > - SET_CXL_REGION_ATTR(region) > > NULL, > > }; > > > > @@ -345,6 +344,7 @@ static const struct attribute_group > > *cxl_decoder_root_attribute_groups[] = { > > static struct attribute *cxl_decoder_switch_attrs[] = { > > &dev_attr_target_type.attr, > > &dev_attr_target_list.attr, > > + SET_CXL_REGION_ATTR(region) > > This patch removes the 'region' attr from switch decoders, but isn't it > also vestigial under root decoders? Oh, never mind, I misread the patch - it adds it to switch and endpoint decoders, removes from root. Reviewed-by: Vishal Verma <vishal.l.verma@intel.com> > > > NULL, > > }; > > > > @@ -364,6 +364,7 @@ static struct attribute > > *cxl_decoder_endpoint_attrs[] = { > > &dev_attr_mode.attr, > > &dev_attr_dpa_size.attr, > > &dev_attr_dpa_resource.attr, > > + SET_CXL_REGION_ATTR(region) > > NULL, > > }; > > > > >
Verma, Vishal L wrote: > On Fri, 2022-07-22 at 17:56 -0700, Dan Williams wrote: > > For switch and endpoint decoders the relationship of decoders to > > regions > > is 1:1. However, for root decoders the relationship is 1:N. Also, > > regions are already children of root decoders, so the 1:N > > relationship > > is observed by walking the following glob: > > > > /sys/bus/cxl/devices/$decoder/region* > > > > Hide the vestigial 'region' attribute for root decoders. > > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > --- > > drivers/cxl/core/port.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > > index 3d2d0119cc3d..bffde862de0b 100644 > > --- a/drivers/cxl/core/port.c > > +++ b/drivers/cxl/core/port.c > > @@ -290,7 +290,6 @@ static struct attribute *cxl_decoder_base_attrs[] > > = { > > &dev_attr_locked.attr, > > &dev_attr_interleave_granularity.attr, > > &dev_attr_interleave_ways.attr, > > - SET_CXL_REGION_ATTR(region) > > NULL, > > }; > > > > @@ -345,6 +344,7 @@ static const struct attribute_group > > *cxl_decoder_root_attribute_groups[] = { > > static struct attribute *cxl_decoder_switch_attrs[] = { > > &dev_attr_target_type.attr, > > &dev_attr_target_list.attr, > > + SET_CXL_REGION_ATTR(region) > > This patch removes the 'region' attr from switch decoders, but isn't it > also vestigial under root decoders? Take another look. It removes it from the common cxl_decoder_base_attrs, and adds it back to just cxl_decoder_switch_attrs and cxl_decoder_endpoint_attrs. Perhaps you read this context info: > > @@ -345,6 +344,7 @@ static const struct attribute_group > > *cxl_decoder_root_attribute_groups[] = { ...and thought it was adding it back to the root decoder attributes?
Verma, Vishal L wrote: > On Mon, 2022-08-01 at 19:38 +0000, Verma, Vishal L wrote: > > On Fri, 2022-07-22 at 17:56 -0700, Dan Williams wrote: > > > For switch and endpoint decoders the relationship of decoders to > > > regions > > > is 1:1. However, for root decoders the relationship is 1:N. Also, > > > regions are already children of root decoders, so the 1:N > > > relationship > > > is observed by walking the following glob: > > > > > > /sys/bus/cxl/devices/$decoder/region* > > > > > > Hide the vestigial 'region' attribute for root decoders. > > > > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > > --- > > > drivers/cxl/core/port.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > > > index 3d2d0119cc3d..bffde862de0b 100644 > > > --- a/drivers/cxl/core/port.c > > > +++ b/drivers/cxl/core/port.c > > > @@ -290,7 +290,6 @@ static struct attribute *cxl_decoder_base_attrs[] > > > = { > > > &dev_attr_locked.attr, > > > &dev_attr_interleave_granularity.attr, > > > &dev_attr_interleave_ways.attr, > > > - SET_CXL_REGION_ATTR(region) > > > NULL, > > > }; > > > > > > @@ -345,6 +344,7 @@ static const struct attribute_group > > > *cxl_decoder_root_attribute_groups[] = { > > > static struct attribute *cxl_decoder_switch_attrs[] = { > > > &dev_attr_target_type.attr, > > > &dev_attr_target_list.attr, > > > + SET_CXL_REGION_ATTR(region) > > > > This patch removes the 'region' attr from switch decoders, but isn't it > > also vestigial under root decoders? > > Oh, never mind, I misread the patch - it adds it to switch and endpoint > decoders, removes from root. Oh, and never mind my previous comment too. > > Reviewed-by: Vishal Verma <vishal.l.verma@intel.com> Thanks!
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 3d2d0119cc3d..bffde862de0b 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -290,7 +290,6 @@ static struct attribute *cxl_decoder_base_attrs[] = { &dev_attr_locked.attr, &dev_attr_interleave_granularity.attr, &dev_attr_interleave_ways.attr, - SET_CXL_REGION_ATTR(region) NULL, }; @@ -345,6 +344,7 @@ static const struct attribute_group *cxl_decoder_root_attribute_groups[] = { static struct attribute *cxl_decoder_switch_attrs[] = { &dev_attr_target_type.attr, &dev_attr_target_list.attr, + SET_CXL_REGION_ATTR(region) NULL, }; @@ -364,6 +364,7 @@ static struct attribute *cxl_decoder_endpoint_attrs[] = { &dev_attr_mode.attr, &dev_attr_dpa_size.attr, &dev_attr_dpa_resource.attr, + SET_CXL_REGION_ATTR(region) NULL, };
For switch and endpoint decoders the relationship of decoders to regions is 1:1. However, for root decoders the relationship is 1:N. Also, regions are already children of root decoders, so the 1:N relationship is observed by walking the following glob: /sys/bus/cxl/devices/$decoder/region* Hide the vestigial 'region' attribute for root decoders. Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/cxl/core/port.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)