diff mbox series

[10/18] cxl/region: Fix passthrough-decoder detection

Message ID 167564540422.847146.13816934143225777888.stgit@dwillia2-xfh.jf.intel.com
State Accepted
Commit 711442e29f16f0d39dd0e2460c9baacfccb9d5a7
Headers show
Series CXL RAM and the 'Soft Reserved' => 'System RAM' default | expand

Commit Message

Dan Williams Feb. 6, 2023, 1:03 a.m. UTC
A passthrough decoder is a decoder that maps only 1 target. It is a
special case because it does not impose any constraints on the
interleave-math as compared to a decoder with multiple targets. Extend
the passthrough case to multi-target-capable decoders that only have one
target selected. I.e. the current code was only considering passthrough
*ports* which are only a subset of the potential passthrough decoder
scenarios.

Fixes: e4f6dfa9ef75 ("cxl/region: Fix 'distance' calculation with passthrough ports")
Cc: <stable@vger.kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/core/region.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Dave Jiang Feb. 7, 2023, midnight UTC | #1
On 2/5/23 6:03 PM, Dan Williams wrote:
> A passthrough decoder is a decoder that maps only 1 target. It is a
> special case because it does not impose any constraints on the
> interleave-math as compared to a decoder with multiple targets. Extend
> the passthrough case to multi-target-capable decoders that only have one
> target selected. I.e. the current code was only considering passthrough
> *ports* which are only a subset of the potential passthrough decoder
> scenarios.
> 
> Fixes: e4f6dfa9ef75 ("cxl/region: Fix 'distance' calculation with passthrough ports")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>   drivers/cxl/core/region.c |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index c82d3b6f3d1f..34cf95217901 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1019,10 +1019,10 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>   		int i, distance;
>   
>   		/*
> -		 * Passthrough ports impose no distance requirements between
> +		 * Passthrough decoders impose no distance requirements between
>   		 * peers
>   		 */
> -		if (port->nr_dports == 1)
> +		if (cxl_rr->nr_targets == 1)
>   			distance = 0;
>   		else
>   			distance = p->nr_targets / cxl_rr->nr_targets;
>
Jonathan Cameron Feb. 8, 2023, 12:44 p.m. UTC | #2
On Sun, 05 Feb 2023 17:03:24 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> A passthrough decoder is a decoder that maps only 1 target. It is a
> special case because it does not impose any constraints on the
> interleave-math as compared to a decoder with multiple targets. Extend
> the passthrough case to multi-target-capable decoders that only have one
> target selected. I.e. the current code was only considering passthrough
> *ports* which are only a subset of the potential passthrough decoder
> scenarios.
> 
> Fixes: e4f6dfa9ef75 ("cxl/region: Fix 'distance' calculation with passthrough ports")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/core/region.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index c82d3b6f3d1f..34cf95217901 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1019,10 +1019,10 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>  		int i, distance;
>  
>  		/*
> -		 * Passthrough ports impose no distance requirements between
> +		 * Passthrough decoders impose no distance requirements between
>  		 * peers

I think we have a terminology inconsistency.  My understanding was we were using
passthrough decoders for the special case where there is no programmable hardware.
In this case I think we are also considering the case where that hardware must
be programmed etc, it's just that we don't care about interleave.
I'd just explain what it is rather than trying to assign a term. 

Decoders that have a single target configured impose...



>  		 */
> -		if (port->nr_dports == 1)
> +		if (cxl_rr->nr_targets == 1)
>  			distance = 0;
>  		else
>  			distance = p->nr_targets / cxl_rr->nr_targets;
> 
>
Verma, Vishal L Feb. 9, 2023, 8:28 p.m. UTC | #3
On Sun, 2023-02-05 at 17:03 -0800, Dan Williams wrote:
> A passthrough decoder is a decoder that maps only 1 target. It is a
> special case because it does not impose any constraints on the
> interleave-math as compared to a decoder with multiple targets. Extend
> the passthrough case to multi-target-capable decoders that only have one
> target selected. I.e. the current code was only considering passthrough
> *ports* which are only a subset of the potential passthrough decoder
> scenarios.
> 
> Fixes: e4f6dfa9ef75 ("cxl/region: Fix 'distance' calculation with passthrough ports")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/core/region.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Looks good,

Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>

> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index c82d3b6f3d1f..34cf95217901 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1019,10 +1019,10 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>                 int i, distance;
>  
>                 /*
> -                * Passthrough ports impose no distance requirements between
> +                * Passthrough decoders impose no distance requirements between
>                  * peers
>                  */
> -               if (port->nr_dports == 1)
> +               if (cxl_rr->nr_targets == 1)
>                         distance = 0;
>                 else
>                         distance = p->nr_targets / cxl_rr->nr_targets;
> 
>
diff mbox series

Patch

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index c82d3b6f3d1f..34cf95217901 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -1019,10 +1019,10 @@  static int cxl_port_setup_targets(struct cxl_port *port,
 		int i, distance;
 
 		/*
-		 * Passthrough ports impose no distance requirements between
+		 * Passthrough decoders impose no distance requirements between
 		 * peers
 		 */
-		if (port->nr_dports == 1)
+		if (cxl_rr->nr_targets == 1)
 			distance = 0;
 		else
 			distance = p->nr_targets / cxl_rr->nr_targets;