Re: [PATCH 08/25] coresight: tmc: Clean up device specific data

[Mathieu Poirier]

Hi Suzuki,

On Wed, Mar 20, 2019 at 06:49:25PM +0000, Suzuki K Poulose wrote:
> In preparation to use a consistent device naming scheme,
> clean up the device link tracking in replicator driver.
> Use the "coresight" device instead of the "real" parent device
> for all internal purposes. All other requests (e.g, power management,
> DMA operations) must use the "real" device which is the parent device.
> 
> Since the CATU driver also uses the TMC-SG infrastructure, update
> the callers to ensure they pass the appropriate device argument
> for the tables.
> 
> Cc: Mathieu Poirier <mathie.poirier@xxxxxxxxxx>

I was wondering why patches 6, 7, and 8 didn't make it to my inbox...  Now I
have the answer :o)

> Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> ---
>  drivers/hwtracing/coresight/coresight-catu.c    |  5 ++---
>  drivers/hwtracing/coresight/coresight-tmc-etr.c | 26 ++++++++++++++-----------
>  drivers/hwtracing/coresight/coresight-tmc.c     | 22 ++++++++++-----------
>  3 files changed, 27 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
> index 170fbb6..11c6f2f 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.c
> +++ b/drivers/hwtracing/coresight/coresight-catu.c
> @@ -328,19 +328,18 @@ static int catu_alloc_etr_buf(struct tmc_drvdata *tmc_drvdata,
>  			      struct etr_buf *etr_buf, int node, void **pages)
>  {
>  	struct coresight_device *csdev;
> -	struct device *catu_dev;
>  	struct tmc_sg_table *catu_table;
>  	struct catu_etr_buf *catu_buf;
>  
>  	csdev = tmc_etr_get_catu_device(tmc_drvdata);
>  	if (!csdev)
>  		return -ENODEV;
> -	catu_dev = csdev->dev.parent;
>  	catu_buf = kzalloc(sizeof(*catu_buf), GFP_KERNEL);
>  	if (!catu_buf)
>  		return -ENOMEM;
>  
> -	catu_table = catu_init_sg_table(catu_dev, node, etr_buf->size, pages);
> +	catu_table = catu_init_sg_table(&csdev->dev, node,
> +					etr_buf->size, pages);
>  	if (IS_ERR(catu_table)) {
>  		kfree(catu_buf);
>  		return PTR_ERR(catu_table);
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index f684283..4152774 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -153,10 +153,11 @@ static void tmc_pages_free(struct tmc_pages *tmc_pages,
>  			   struct device *dev, enum dma_data_direction dir)
>  {
>  	int i;
> +	struct device *real_dev = dev->parent;
>  
>  	for (i = 0; i < tmc_pages->nr_pages; i++) {
>  		if (tmc_pages->daddrs && tmc_pages->daddrs[i])
> -			dma_unmap_page(dev, tmc_pages->daddrs[i],
> +			dma_unmap_page(real_dev, tmc_pages->daddrs[i],
>  					 PAGE_SIZE, dir);
>  		if (tmc_pages->pages && tmc_pages->pages[i])
>  			__free_page(tmc_pages->pages[i]);
> @@ -184,6 +185,7 @@ static int tmc_pages_alloc(struct tmc_pages *tmc_pages,
>  	int i, nr_pages;
>  	dma_addr_t paddr;
>  	struct page *page;
> +	struct device *real_dev = dev->parent;
>  
>  	nr_pages = tmc_pages->nr_pages;
>  	tmc_pages->daddrs = kcalloc(nr_pages, sizeof(*tmc_pages->daddrs),
> @@ -207,8 +209,8 @@ static int tmc_pages_alloc(struct tmc_pages *tmc_pages,
>  			page = alloc_pages_node(node,
>  						GFP_KERNEL | __GFP_ZERO, 0);
>  		}
> -		paddr = dma_map_page(dev, page, 0, PAGE_SIZE, dir);
> -		if (dma_mapping_error(dev, paddr))
> +		paddr = dma_map_page(real_dev, page, 0, PAGE_SIZE, dir);
> +		if (dma_mapping_error(real_dev, paddr))
>  			goto err;
>  		tmc_pages->daddrs[i] = paddr;
>  		tmc_pages->pages[i] = page;
> @@ -295,7 +297,7 @@ static int tmc_alloc_data_pages(struct tmc_sg_table *sg_table, void **pages)
>   * and data buffers. TMC writes to the data buffers and reads from the SG
>   * Table pages.
>   *
> - * @dev		- Device to which page should be DMA mapped.
> + * @dev		- Coresight device to which page should be DMA mapped.
>   * @node	- Numa node for mem allocations
>   * @nr_tpages	- Number of pages for the table entries.
>   * @nr_dpages	- Number of pages for Data buffer.
> @@ -339,13 +341,13 @@ void tmc_sg_table_sync_data_range(struct tmc_sg_table *table,
>  {
>  	int i, index, start;
>  	int npages = DIV_ROUND_UP(size, PAGE_SIZE);
> -	struct device *dev = table->dev;
> +	struct device *real_dev = table->dev->parent;
>  	struct tmc_pages *data = &table->data_pages;
>  
>  	start = offset >> PAGE_SHIFT;
>  	for (i = start; i < (start + npages); i++) {
>  		index = i % data->nr_pages;
> -		dma_sync_single_for_cpu(dev, data->daddrs[index],
> +		dma_sync_single_for_cpu(real_dev, data->daddrs[index],
>  					PAGE_SIZE, DMA_FROM_DEVICE);
>  	}
>  }
> @@ -354,11 +356,11 @@ void tmc_sg_table_sync_data_range(struct tmc_sg_table *table,
>  void tmc_sg_table_sync_table(struct tmc_sg_table *sg_table)
>  {
>  	int i;
> -	struct device *dev = sg_table->dev;
> +	struct device *real_dev = sg_table->dev->parent;
>  	struct tmc_pages *table_pages = &sg_table->table_pages;
>  
>  	for (i = 0; i < table_pages->nr_pages; i++)
> -		dma_sync_single_for_device(dev, table_pages->daddrs[i],
> +		dma_sync_single_for_device(real_dev, table_pages->daddrs[i],
>  					   PAGE_SIZE, DMA_TO_DEVICE);
>  }
>  
> @@ -581,6 +583,7 @@ static int tmc_etr_alloc_flat_buf(struct tmc_drvdata *drvdata,
>  				  void **pages)
>  {
>  	struct etr_flat_buf *flat_buf;
> +	struct device *real_dev = drvdata->dev->parent;
>  
>  	/* We cannot reuse existing pages for flat buf */
>  	if (pages)
> @@ -590,7 +593,7 @@ static int tmc_etr_alloc_flat_buf(struct tmc_drvdata *drvdata,
>  	if (!flat_buf)
>  		return -ENOMEM;
>  
> -	flat_buf->vaddr = dma_alloc_coherent(drvdata->dev, etr_buf->size,
> +	flat_buf->vaddr = dma_alloc_coherent(real_dev, etr_buf->size,
>  					     &flat_buf->daddr, GFP_KERNEL);
>  	if (!flat_buf->vaddr) {
>  		kfree(flat_buf);
> @@ -608,9 +611,10 @@ static int tmc_etr_alloc_flat_buf(struct tmc_drvdata *drvdata,
>  static void tmc_etr_free_flat_buf(struct etr_buf *etr_buf)
>  {
>  	struct etr_flat_buf *flat_buf = etr_buf->private;
> +	struct device *real_dev = flat_buf->dev->parent;
>  
>  	if (flat_buf && flat_buf->daddr)
> -		dma_free_coherent(flat_buf->dev, flat_buf->size,
> +		dma_free_coherent(real_dev, flat_buf->size,
>  				  flat_buf->vaddr, flat_buf->daddr);
>  	kfree(flat_buf);
>  }
> @@ -815,7 +819,7 @@ static struct etr_buf *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata,
>  	struct etr_buf *etr_buf;
>  
>  	has_etr_sg = tmc_etr_has_cap(drvdata, TMC_ETR_SG);
> -	has_iommu = iommu_get_domain_for_dev(drvdata->dev);
> +	has_iommu = iommu_get_domain_for_dev(drvdata->dev->parent);
>  	has_catu = !!tmc_etr_get_catu_device(drvdata);
>  
>  	has_sg = has_catu || has_etr_sg;
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
> index 647b6aa..9e7ff83 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
> @@ -330,24 +330,22 @@ const struct attribute_group *coresight_tmc_groups[] = {
>  	NULL,
>  };
>  
> -static inline bool tmc_etr_can_use_sg(struct tmc_drvdata *drvdata)
> +static inline bool tmc_etr_can_use_sg(struct device *dev)
>  {
> -	return fwnode_property_present(drvdata->dev->fwnode,
> -				       "arm,scatter-gather");
> +	return fwnode_property_present(dev->fwnode, "arm,scatter-gather");
>  }
>  
>  /* Detect and initialise the capabilities of a TMC ETR */
> -static int tmc_etr_setup_caps(struct tmc_drvdata *drvdata,
> -			     u32 devid, void *dev_caps)
> +static int tmc_etr_setup_caps(struct device *parent, u32 devid, void *dev_caps)
>  {
>  	int rc;
> -
>  	u32 dma_mask = 0;
> +	struct tmc_drvdata *drvdata = dev_get_drvdata(parent);
>  
>  	/* Set the unadvertised capabilities */
>  	tmc_etr_init_caps(drvdata, (u32)(unsigned long)dev_caps);
>  
> -	if (!(devid & TMC_DEVID_NOSCAT) && tmc_etr_can_use_sg(drvdata))
> +	if (!(devid & TMC_DEVID_NOSCAT) && tmc_etr_can_use_sg(parent))
>  		tmc_etr_set_cap(drvdata, TMC_ETR_SG);
>  
>  	/* Check if the AXI address width is available */
> @@ -365,15 +363,15 @@ static int tmc_etr_setup_caps(struct tmc_drvdata *drvdata,
>  	case 44:
>  	case 48:
>  	case 52:
> -		dev_info(drvdata->dev, "Detected dma mask %dbits\n", dma_mask);
> +		dev_info(parent, "Detected dma mask %dbits\n", dma_mask);
>  		break;
>  	default:
>  		dma_mask = 40;
>  	}
>  
> -	rc = dma_set_mask_and_coherent(drvdata->dev, DMA_BIT_MASK(dma_mask));
> +	rc = dma_set_mask_and_coherent(parent, DMA_BIT_MASK(dma_mask));
>  	if (rc)
> -		dev_err(drvdata->dev, "Failed to setup DMA mask: %d\n", rc);
> +		dev_err(parent, "Failed to setup DMA mask: %d\n", rc);
>  	return rc;
>  }
>  
> @@ -403,7 +401,6 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
>  	if (!drvdata)
>  		goto out;
>  
> -	drvdata->dev = &adev->dev;
>  	dev_set_drvdata(dev, drvdata);
>  
>  	/* Validity for the resource is already checked by the AMBA core */
> @@ -448,7 +445,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
>  		desc.type = CORESIGHT_DEV_TYPE_SINK;
>  		desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
>  		desc.ops = &tmc_etr_cs_ops;
> -		ret = tmc_etr_setup_caps(drvdata, devid,
> +		ret = tmc_etr_setup_caps(dev, devid,
>  					 coresight_get_uci_data(id));
>  		if (ret)
>  			goto out;
> @@ -470,6 +467,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
>  		goto out;
>  	}
>  
> +	drvdata->dev = &drvdata->csdev->dev;

Any reason for not getting rid of tmc_drvdata::dev like you did for the funnel,
replicator and catu?

>  	drvdata->miscdev.name = pdata->name;
>  	drvdata->miscdev.minor = MISC_DYNAMIC_MINOR;
>  	drvdata->miscdev.fops = &tmc_fops;
> -- 
> 2.7.4
>