Messages in this thread | | | Subject | Re: [PATCH v3 5/8] drivers/soc: xdma: Add PCI device configuration sysfs | From | Eddie James <> | Date | Mon, 1 Jul 2019 13:38:56 -0500 |
| |
On 5/30/19 10:45 PM, Eduardo Valentin wrote: > On Wed, May 29, 2019 at 01:10:05PM -0500, Eddie James wrote: >> The AST2500 has two PCI devices embedded. The XDMA engine can use either >> device to perform DMA transfers. Users need the capability to choose >> which device to use. This commit therefore adds two sysfs files that >> toggle the AST2500 and XDMA engine between the two PCI devices. >> >> Signed-off-by: Eddie James <eajames@linux.ibm.com> >> --- >> drivers/soc/aspeed/aspeed-xdma.c | 103 +++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 100 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/soc/aspeed/aspeed-xdma.c b/drivers/soc/aspeed/aspeed-xdma.c >> index 39f6545..ddd5e1e 100644 >> --- a/drivers/soc/aspeed/aspeed-xdma.c >> +++ b/drivers/soc/aspeed/aspeed-xdma.c >> @@ -143,6 +143,7 @@ struct aspeed_xdma { >> void *cmdq_vga_virt; >> struct gen_pool *vga_pool; >> >> + char pcidev[4]; >> struct miscdevice misc; >> }; >> >> @@ -165,6 +166,10 @@ struct aspeed_xdma_client { >> SCU_PCIE_CONF_VGA_EN_IRQ | SCU_PCIE_CONF_VGA_EN_DMA | >> SCU_PCIE_CONF_RSVD; >> >> +static char *_pcidev = "vga"; >> +module_param_named(pcidev, _pcidev, charp, 0600); >> +MODULE_PARM_DESC(pcidev, "Default PCI device used by XDMA engine for DMA ops"); >> + >> static void aspeed_scu_pcie_write(struct aspeed_xdma *ctx, u32 conf) >> { >> u32 v = 0; >> @@ -512,7 +517,7 @@ static int aspeed_xdma_release(struct inode *inode, struct file *file) >> .release = aspeed_xdma_release, >> }; >> >> -static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx) >> +static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx, u32 conf) >> { >> int rc; >> u32 scu_conf = 0; >> @@ -522,7 +527,7 @@ static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx) >> const u32 vga_sizes[4] = { 0x800000, 0x1000000, 0x2000000, 0x4000000 }; >> void __iomem *sdmc_base = ioremap(0x1e6e0000, 0x100); >> >> - aspeed_scu_pcie_write(ctx, aspeed_xdma_vga_pcie_conf); >> + aspeed_scu_pcie_write(ctx, conf); >> >> regmap_read(ctx->scu, SCU_STRAP, &scu_conf); >> ctx->vga_size = vga_sizes[FIELD_GET(SCU_STRAP_VGA_MEM, scu_conf)]; >> @@ -598,10 +603,91 @@ static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx) >> return rc; >> } >> >> +static int aspeed_xdma_change_pcie_conf(struct aspeed_xdma *ctx, u32 conf) >> +{ >> + int rc; >> + >> + mutex_lock(&ctx->start_lock); >> + rc = wait_event_interruptible_timeout(ctx->wait, >> + !test_bit(XDMA_IN_PRG, >> + &ctx->flags), >> + msecs_to_jiffies(1000)); >> + if (rc < 0) { >> + mutex_unlock(&ctx->start_lock); >> + return -EINTR; >> + } >> + >> + /* previous op didn't complete, wake up waiters anyway */ >> + if (!rc) >> + wake_up_interruptible_all(&ctx->wait); >> + >> + reset_control_assert(ctx->reset); >> + msleep(10); >> + >> + aspeed_scu_pcie_write(ctx, conf); >> + msleep(10); >> + >> + reset_control_deassert(ctx->reset); >> + msleep(10); >> + >> + aspeed_xdma_init_eng(ctx); >> + >> + mutex_unlock(&ctx->start_lock); >> + >> + return 0; >> +} >> + >> +static int aspeed_xdma_pcidev_to_conf(struct aspeed_xdma *ctx, >> + const char *pcidev, u32 *conf) >> +{ >> + if (!strcasecmp(pcidev, "vga")) { >> + *conf = aspeed_xdma_vga_pcie_conf; >> + return 0; >> + } >> + >> + if (!strcasecmp(pcidev, "bmc")) { >> + *conf = aspeed_xdma_bmc_pcie_conf; >> + return 0; >> + } > strncasecmp()?
Yes, will change, and below.
> >> + >> + return -EINVAL; >> +} >> + >> +static ssize_t aspeed_xdma_show_pcidev(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct aspeed_xdma *ctx = dev_get_drvdata(dev); >> + >> + return snprintf(buf, PAGE_SIZE - 1, "%s", ctx->pcidev); >> +} >> + >> +static ssize_t aspeed_xdma_store_pcidev(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + u32 conf; >> + struct aspeed_xdma *ctx = dev_get_drvdata(dev); >> + int rc = aspeed_xdma_pcidev_to_conf(ctx, buf, &conf); >> + >> + if (rc) >> + return rc; >> + >> + rc = aspeed_xdma_change_pcie_conf(ctx, conf); >> + if (rc) >> + return rc; >> + >> + strcpy(ctx->pcidev, buf); > should we use strncpy() instead? > >> + return count; >> +} >> +static DEVICE_ATTR(pcidev, 0644, aspeed_xdma_show_pcidev, >> + aspeed_xdma_store_pcidev); >> + >> static int aspeed_xdma_probe(struct platform_device *pdev) >> { >> int irq; >> int rc; >> + u32 conf; >> struct resource *res; >> struct device *dev = &pdev->dev; >> struct aspeed_xdma *ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); >> @@ -657,7 +743,14 @@ static int aspeed_xdma_probe(struct platform_device *pdev) >> >> msleep(10); >> >> - rc = aspeed_xdma_init_mem(ctx); >> + if (aspeed_xdma_pcidev_to_conf(ctx, _pcidev, &conf)) { >> + conf = aspeed_xdma_vga_pcie_conf; >> + strcpy(ctx->pcidev, "vga"); >> + } else { >> + strcpy(ctx->pcidev, _pcidev); >> + } > same... > >> + >> + rc = aspeed_xdma_init_mem(ctx, conf); >> if (rc) { >> reset_control_assert(ctx->reset); >> return rc; >> @@ -682,6 +775,8 @@ static int aspeed_xdma_probe(struct platform_device *pdev) >> return rc; >> } >> >> + device_create_file(dev, &dev_attr_pcidev); > Should we consider using one of the default attributes here instead of device_create_file()? > http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
Doesn't seem to be any way to create attributes per device with that method. Setting the device->groups in probe() doesn't do it.
> > BTW, was this ABI documented? Is this the same file documented in patch 2?
Patch 4, but yes.
> >> + >> return 0; >> } >> >> @@ -689,6 +784,8 @@ static int aspeed_xdma_remove(struct platform_device *pdev) >> { >> struct aspeed_xdma *ctx = platform_get_drvdata(pdev); >> >> + device_remove_file(ctx->dev, &dev_attr_pcidev); >> + >> misc_deregister(&ctx->misc); >> gen_pool_free(ctx->vga_pool, (unsigned long)ctx->cmdq_vga_virt, >> XDMA_CMDQ_SIZE); >> -- >> 1.8.3.1 >>
| |