On 17/09/25 04:55PM, Jonathan Cameron wrote:
On Tue, 16 Sep 2025 13:37:35 +0530
Arpit Kumar <arpit1.ku...@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.ku...@samsung.com>

Hi Arpit.  One question inline, and one comment on code I've moved
around whilst queue this up.  I'll push out a tree to gitlab
(probably tomorrow) and when I do please check I didn't mess that up!

Jonathan


Hi Jonathan,
Thank you for the swift response and review comments.
Sure, will look into gitlab tree once up.

Thanks,
Arpit

+static CXLRetCode cxl_set_port_type(CXLUpstreamPort *ports, int pnum,
+                                    CXLCCI *cci)
+{
+    uint8_t current_port_config_state;
+    uint8_t connected_device_type;
+    uint8_t supported_ld_count;
+    uint16_t lnkcap, lnkcap2, lnksta;
+    PCIBus *bus;
+    PCIDevice *port_dev;
+    PCIEPort *usp = PCIE_PORT(cci->d);
+
+    if (usp->port == pnum) {
+        port_dev = PCI_DEVICE(usp);
+        current_port_config_state = CXL_PORT_CONFIG_STATE_USP;
+        connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_NONE;
+        supported_ld_count = 0;
+    } else {
+        bus = &PCI_BRIDGE(cci->d)->sec_bus;
+        port_dev = pcie_find_port_by_pn(bus, pnum);
+        if (port_dev) { /* DSP */
+            PCIDevice *ds_dev = pci_bridge_get_sec_bus(PCI_BRIDGE(port_dev))
+                ->devices[0];
+            current_port_config_state = CXL_PORT_CONFIG_STATE_DSP;
+            if (ds_dev) {
+                if (object_dynamic_cast(OBJECT(ds_dev), TYPE_CXL_TYPE3)) {
+                    /* To-do: controllable */

In what sense controllable?  It should always match what the downstream device
is presenting as.  Do you ultimately mean if we mess with the alternate modes
and reset the port to have it come up as a PCI only device?
This will need to be more complex as we add different CXL type 3 device support
of course, but I'd still expect to auto detect it rather that control it 
directly.


This is with respect to your review comment from v1 patch:
https://lore.kernel.org/qemu-devel/20250602135942.2773823-1-arpit1.ku...@samsung.com/T/
As per my understanding, controllable was identification of the specific type of CXL type 3 device and accordingly initializing connected_device_type. Since you mention auto-detect, does it mean using object_get_typename() to identify the type
of device and initiliaze it directly to connected_device_type rather than 
specifying
it explicitly. If yes, then this anyways rules out controllable part, hence the 
comment
can be removed.

+                    connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_3_SLD;
+                } else {
+                    connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_PCIE;
+                }
+            } else {
+                connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_NONE;
+            }
+            supported_ld_count = 3;
+        } else {
+            return CXL_MBOX_INVALID_INPUT;
+        }
+    }

 void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
                                   DeviceState *d, size_t payload_max)
 {
@@ -4691,6 +4706,7 @@ void cxl_initialize_mailbox_swcci(CXLCCI *cci, 
DeviceState *intf,
     cci->d = d;
     cci->intf = intf;
     cxl_init_cci(cci, payload_max);
+    cxl_set_phy_port_info(cci);
 }

 void cxl_initialize_mailbox_t3(CXLCCI *cci, DeviceState *d, size_t payload_max)
@@ -4777,4 +4793,5 @@ void cxl_initialize_usp_mctpcci(CXLCCI *cci, DeviceState 
*d, DeviceState *intf,
     cci->d = d;
     cci->intf = intf;
     cxl_init_cci(cci, payload_max);
+    cxl_set_phy_port_info(cci);

I'll shift this to a later patch whilst picking this up for my staging tree.
I want this ahead of where we introduce cxl_initialize_usp_mctpcci.

Okay
 }


Reply via email to