Re: [PATCH v3 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Mon, 8 Sep 2025 19:18:56 +0530
Arpit Kumar <arpit1.kumar@samsung.com> wrote:

> On 05/09/25 04:59PM, Jonathan Cameron wrote:
> >On Thu,  4 Sep 2025 18:49:03 +0530
> >Arpit Kumar <arpit1.kumar@samsung.com> wrote:
> >  
> >> -Storing physical ports info during enumeration.
> >> -Refactored changes using physical ports info for
> >>  Identify Switch Device (Opcode 5100h) & Get Physical Port State
> >>  (Opcode 5101h) physical switch FM-API command set.
> >>
> >> Signed-off-by: Arpit Kumar <arpit1.kumar@samsung.com>  
> >Hi Arpit,
> >
> >A few minor things inline.   Biggest one is making sure
> >to namespace the defines which is quite challenging to do
> >here without a really long name.
> >
> >Jonathan
> >  
> Hi Jonathan,
> Thanks for the review! Will update the changes in next 
> iteration v4 of the patch-set.

> >> -        port->connected_device_cxl_version = 0x2;
> >> +        memcpy(&out->ports[i], &(pp->pports.pport_info[pn]),
> >> +               sizeof(CXLPhyPortInfo));
> >>      }
> >> -  
> >
> >That's a stray change that shouldn't be here.

I wasn't clear enough. I just meant that final line removal, not the rest
of the code! Sorry for confusion.

> >  
> Correct me if I am wrong but the current initializations for the ports are
> moved to cxl_set_port_type(), hence this might appear stray in this case due
> to eliminations but this is with respect to the response payload of
> cmd_get_physical_port_state() so seems apt.
> However, one change required would be to move: out->num_ports = in->num_ports; after
> the below for loop as it validates the port_id's:
>      for (i = 0; i < in->num_ports; i++) {
>          int pn = in->ports[i];
> 
>          if (pp->pports.pport_info[pn].port_id != pn) {
>              return CXL_MBOX_INVALID_INPUT;
>          }
>          memcpy(&out->ports[i], &(pp->pports.pport_info[pn]),
>                 sizeof(CXLPhyPortInfo));
>      }