diff mbox series

[net-next,v3,2/5] bus: fsl-mc: add the fsl_mc_get_endpoint function

Message ID 1571998630-17108-3-git-send-email-ioana.ciornei@nxp.com
State Changes Requested
Delegated to: David Miller
Headers show
Series dpaa2-eth: add MAC/PHY support through phylink | expand

Commit Message

Ioana Ciornei Oct. 25, 2019, 10:17 a.m. UTC 
Using the newly added fsl_mc_get_endpoint function a fsl-mc driver can
find its associated endpoint (another object at the other link of a MC
firmware link).

The API will be used in the following patch in order to discover the
connected DPMAC object of a DPNI.

Also, the fsl_mc_device_lookup function is made available to the entire
fsl-mc bus driver and not just for the dprc driver.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v2:
 - use ENOTCONN instead of EINVAL
Changes in v3:
 - none

 drivers/bus/fsl-mc/dprc-driver.c    |  6 ++---
 drivers/bus/fsl-mc/dprc.c           | 53 +++++++++++++++++++++++++++++++++++++
 drivers/bus/fsl-mc/fsl-mc-bus.c     | 33 +++++++++++++++++++++++
 drivers/bus/fsl-mc/fsl-mc-private.h | 42 +++++++++++++++++++++++++++++
 include/linux/fsl/mc.h              |  2 ++
 5 files changed, 132 insertions(+), 4 deletions(-)

Comments

Andrew Lunn Oct. 29, 2019, 1:45 a.m. UTC | #1 
> +struct fsl_mc_device *fsl_mc_get_endpoint(struct fsl_mc_device *mc_dev)
> +{
> +	struct fsl_mc_device *mc_bus_dev, *endpoint;
> +	struct fsl_mc_obj_desc endpoint_desc = { 0 };
> +	struct dprc_endpoint endpoint1 = { 0 };
> +	struct dprc_endpoint endpoint2 = { 0 };
> +	int state, err;
> +
> +	mc_bus_dev = to_fsl_mc_device(mc_dev->dev.parent);
> +	strcpy(endpoint1.type, mc_dev->obj_desc.type);
> +	endpoint1.id = mc_dev->obj_desc.id;
> +
> +	err = dprc_get_connection(mc_bus_dev->mc_io, 0,
> +				  mc_bus_dev->mc_handle,
> +				  &endpoint1, &endpoint2,
> +				  &state);
> +
> +	if (err == -ENOTCONN || state == -1)
> +		return NULL;
> +
> +	if (err < 0) {
> +		dev_err(&mc_bus_dev->dev, "dprc_get_connection() = %d\n", err);
> +		return NULL;
> +	}

You could return these errors with ERR_PRT(err)

    Andrew
Ioana Ciornei Oct. 29, 2019, 9:26 a.m. UTC | #2 
> Subject: Re: [PATCH net-next v3 2/5] bus: fsl-mc: add the fsl_mc_get_endpoint
> function
> 
> > +struct fsl_mc_device *fsl_mc_get_endpoint(struct fsl_mc_device
> > +*mc_dev) {
> > +	struct fsl_mc_device *mc_bus_dev, *endpoint;
> > +	struct fsl_mc_obj_desc endpoint_desc = { 0 };
> > +	struct dprc_endpoint endpoint1 = { 0 };
> > +	struct dprc_endpoint endpoint2 = { 0 };
> > +	int state, err;
> > +
> > +	mc_bus_dev = to_fsl_mc_device(mc_dev->dev.parent);
> > +	strcpy(endpoint1.type, mc_dev->obj_desc.type);
> > +	endpoint1.id = mc_dev->obj_desc.id;
> > +
> > +	err = dprc_get_connection(mc_bus_dev->mc_io, 0,
> > +				  mc_bus_dev->mc_handle,
> > +				  &endpoint1, &endpoint2,
> > +				  &state);
> > +
> > +	if (err == -ENOTCONN || state == -1)
> > +		return NULL;
> > +
> > +	if (err < 0) {
> > +		dev_err(&mc_bus_dev->dev, "dprc_get_connection() = %d\n",
> err);
> > +		return NULL;
> > +	}
> 
> You could return these errors with ERR_PRT(err)
> 
>     Andrew

I could but the caller would still take the same action disregarding the actual error code.
I don't find it really useful to return ERR_PTR().

Ioana
David Miller Oct. 30, 2019, 9:59 p.m. UTC | #3 
From: Ioana Ciornei <ioana.ciornei@nxp.com>
Date: Fri, 25 Oct 2019 13:17:07 +0300

> @@ -712,6 +712,39 @@ void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
>  }
>  EXPORT_SYMBOL_GPL(fsl_mc_device_remove);
>  
> +struct fsl_mc_device *fsl_mc_get_endpoint(struct fsl_mc_device *mc_dev)
> +{

Like Andrew, I'd really like to see this returning error pointers.

Even if right now the callers don't really change their actions in any
way for different error types.  It's just so much clearer with error
pointers and opens up propagation of error codes in the future if that
works.

Thank you.
Ioana Ciornei Oct. 30, 2019, 10:26 p.m. UTC | #4 
On 10/30/19 11:59 PM, David Miller wrote:
> From: Ioana Ciornei <ioana.ciornei@nxp.com>
> Date: Fri, 25 Oct 2019 13:17:07 +0300
> 
>> @@ -712,6 +712,39 @@ void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
>>   }
>>   EXPORT_SYMBOL_GPL(fsl_mc_device_remove);
>>   
>> +struct fsl_mc_device *fsl_mc_get_endpoint(struct fsl_mc_device *mc_dev)
>> +{
> 
> Like Andrew, I'd really like to see this returning error pointers.
> 
> Even if right now the callers don't really change their actions in any
> way for different error types.  It's just so much clearer with error
> pointers and opens up propagation of error codes in the future if that
> works.
> 
> Thank you.
> 


Sure, no problem. I'll submit a v4 shortly.

Thanks,
Ioana
diff mbox series

Patch

diff --git a/drivers/bus/fsl-mc/dprc-driver.c b/drivers/bus/fsl-mc/dprc-driver.c
index 52c7e15143d6..c8b1c3842c1a 100644
--- a/drivers/bus/fsl-mc/dprc-driver.c
+++ b/drivers/bus/fsl-mc/dprc-driver.c
@@ -104,10 +104,8 @@  static int __fsl_mc_device_match(struct device *dev, void *data)
 	return fsl_mc_device_match(mc_dev, obj_desc);
 }
 
-static struct fsl_mc_device *fsl_mc_device_lookup(struct fsl_mc_obj_desc
-								*obj_desc,
-						  struct fsl_mc_device
-								*mc_bus_dev)
+struct fsl_mc_device *fsl_mc_device_lookup(struct fsl_mc_obj_desc *obj_desc,
+					   struct fsl_mc_device *mc_bus_dev)
 {
 	struct device *dev;
 
diff --git a/drivers/bus/fsl-mc/dprc.c b/drivers/bus/fsl-mc/dprc.c
index 0fe3f52ae0de..602f030d84eb 100644
--- a/drivers/bus/fsl-mc/dprc.c
+++ b/drivers/bus/fsl-mc/dprc.c
@@ -554,3 +554,56 @@  int dprc_get_container_id(struct fsl_mc_io *mc_io,
 
 	return 0;
 }
+
+/**
+ * dprc_get_connection() - Get connected endpoint and link status if connection
+ *			exists.
+ * @mc_io:	Pointer to MC portal's I/O object
+ * @cmd_flags:	Command flags; one or more of 'MC_CMD_FLAG_'
+ * @token:	Token of DPRC object
+ * @endpoint1:	Endpoint 1 configuration parameters
+ * @endpoint2:	Returned endpoint 2 configuration parameters
+ * @state:	Returned link state:
+ *		1 - link is up;
+ *		0 - link is down;
+ *		-1 - no connection (endpoint2 information is irrelevant)
+ *
+ * Return:     '0' on Success; -ENOTCONN if connection does not exist.
+ */
+int dprc_get_connection(struct fsl_mc_io *mc_io,
+			u32 cmd_flags,
+			u16 token,
+			const struct dprc_endpoint *endpoint1,
+			struct dprc_endpoint *endpoint2,
+			int *state)
+{
+	struct dprc_cmd_get_connection *cmd_params;
+	struct dprc_rsp_get_connection *rsp_params;
+	struct fsl_mc_command cmd = { 0 };
+	int err, i;
+
+	/* prepare command */
+	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_CONNECTION,
+					  cmd_flags,
+					  token);
+	cmd_params = (struct dprc_cmd_get_connection *)cmd.params;
+	cmd_params->ep1_id = cpu_to_le32(endpoint1->id);
+	cmd_params->ep1_interface_id = cpu_to_le16(endpoint1->if_id);
+	for (i = 0; i < 16; i++)
+		cmd_params->ep1_type[i] = endpoint1->type[i];
+
+	/* send command to mc */
+	err = mc_send_command(mc_io, &cmd);
+	if (err)
+		return -ENOTCONN;
+
+	/* retrieve response parameters */
+	rsp_params = (struct dprc_rsp_get_connection *)cmd.params;
+	endpoint2->id = le32_to_cpu(rsp_params->ep2_id);
+	endpoint2->if_id = le16_to_cpu(rsp_params->ep2_interface_id);
+	*state = le32_to_cpu(rsp_params->state);
+	for (i = 0; i < 16; i++)
+		endpoint2->type[i] = rsp_params->ep2_type[i];
+
+	return 0;
+}
diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index bb3c2fc7c5ba..6412d887b36b 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -712,6 +712,39 @@  void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
 }
 EXPORT_SYMBOL_GPL(fsl_mc_device_remove);
 
+struct fsl_mc_device *fsl_mc_get_endpoint(struct fsl_mc_device *mc_dev)
+{
+	struct fsl_mc_device *mc_bus_dev, *endpoint;
+	struct fsl_mc_obj_desc endpoint_desc = { 0 };
+	struct dprc_endpoint endpoint1 = { 0 };
+	struct dprc_endpoint endpoint2 = { 0 };
+	int state, err;
+
+	mc_bus_dev = to_fsl_mc_device(mc_dev->dev.parent);
+	strcpy(endpoint1.type, mc_dev->obj_desc.type);
+	endpoint1.id = mc_dev->obj_desc.id;
+
+	err = dprc_get_connection(mc_bus_dev->mc_io, 0,
+				  mc_bus_dev->mc_handle,
+				  &endpoint1, &endpoint2,
+				  &state);
+
+	if (err == -ENOTCONN || state == -1)
+		return NULL;
+
+	if (err < 0) {
+		dev_err(&mc_bus_dev->dev, "dprc_get_connection() = %d\n", err);
+		return NULL;
+	}
+
+	strcpy(endpoint_desc.type, endpoint2.type);
+	endpoint_desc.id = endpoint2.id;
+	endpoint = fsl_mc_device_lookup(&endpoint_desc, mc_bus_dev);
+
+	return endpoint;
+}
+EXPORT_SYMBOL_GPL(fsl_mc_get_endpoint);
+
 static int parse_mc_ranges(struct device *dev,
 			   int *paddr_cells,
 			   int *mc_addr_cells,
diff --git a/drivers/bus/fsl-mc/fsl-mc-private.h b/drivers/bus/fsl-mc/fsl-mc-private.h
index 020fcc04ec8b..21ca8c756ee7 100644
--- a/drivers/bus/fsl-mc/fsl-mc-private.h
+++ b/drivers/bus/fsl-mc/fsl-mc-private.h
@@ -105,6 +105,8 @@  int dpmcp_reset(struct fsl_mc_io *mc_io,
 #define DPRC_CMDID_GET_OBJ_REG_V2               DPRC_CMD_V2(0x15E)
 #define DPRC_CMDID_SET_OBJ_IRQ                  DPRC_CMD(0x15F)
 
+#define DPRC_CMDID_GET_CONNECTION               DPRC_CMD(0x16C)
+
 struct dprc_cmd_open {
 	__le32 container_id;
 };
@@ -228,6 +230,22 @@  struct dprc_cmd_set_obj_irq {
 	u8 obj_type[16];
 };
 
+struct dprc_cmd_get_connection {
+	__le32 ep1_id;
+	__le16 ep1_interface_id;
+	u8 pad[2];
+	u8 ep1_type[16];
+};
+
+struct dprc_rsp_get_connection {
+	__le64 pad[3];
+	__le32 ep2_id;
+	__le16 ep2_interface_id;
+	__le16 pad1;
+	u8 ep2_type[16];
+	__le32 state;
+};
+
 /*
  * DPRC API for managing and querying DPAA resources
  */
@@ -392,6 +410,27 @@  int dprc_get_container_id(struct fsl_mc_io *mc_io,
 			  u32 cmd_flags,
 			  int *container_id);
 
+/**
+ * struct dprc_endpoint - Endpoint description for link connect/disconnect
+ *			operations
+ * @type:	Endpoint object type: NULL terminated string
+ * @id:		Endpoint object ID
+ * @if_id:	Interface ID; should be set for endpoints with multiple
+ *		interfaces ("dpsw", "dpdmux"); for others, always set to 0
+ */
+struct dprc_endpoint {
+	char type[16];
+	int id;
+	u16 if_id;
+};
+
+int dprc_get_connection(struct fsl_mc_io *mc_io,
+			u32 cmd_flags,
+			u16 token,
+			const struct dprc_endpoint *endpoint1,
+			struct dprc_endpoint *endpoint2,
+			int *state);
+
 /*
  * Data Path Buffer Pool (DPBP) API
  */
@@ -574,4 +613,7 @@  int __must_check fsl_create_mc_io(struct device *dev,
 
 bool fsl_mc_is_root_dprc(struct device *dev);
 
+struct fsl_mc_device *fsl_mc_device_lookup(struct fsl_mc_obj_desc *obj_desc,
+					   struct fsl_mc_device *mc_bus_dev);
+
 #endif /* _FSL_MC_PRIVATE_H_ */
diff --git a/include/linux/fsl/mc.h b/include/linux/fsl/mc.h
index 975553a9f75d..54d9436600c7 100644
--- a/include/linux/fsl/mc.h
+++ b/include/linux/fsl/mc.h
@@ -403,6 +403,8 @@  struct irq_domain *fsl_mc_msi_create_irq_domain(struct fwnode_handle *fwnode,
 
 void fsl_mc_free_irqs(struct fsl_mc_device *mc_dev);
 
+struct fsl_mc_device *fsl_mc_get_endpoint(struct fsl_mc_device *mc_dev);
+
 extern struct bus_type fsl_mc_bus_type;
 
 extern struct device_type fsl_mc_bus_dprc_type;