[PATCH V7 2/3] dma: add Qualcomm Technologies HIDMA management driver
From: Vinod Koul <hidden>
Date: 2015-11-30 08:18:49
Also in:
linux-arm-msm, lkml
On Sun, Nov 22, 2015 at 09:28:24PM -0500, Sinan Kaya wrote:
quoted hunk ↗ jump to hunk
+++ b/Documentation/ABI/testing/sysfs-platform-hidma-mgmt@@ -0,0 +1,97 @@ +What: /sys/devices/platform/hidma-mgmt*/chan*/priority + /sys/devices/platform/QCOM8060:*/chan*/priority +Date: Nov 2015 +KernelVersion: 4.4 +Contact: "Sinan Kaya <okaya@cudeaurora.org>" +Description: + Contains either 0 or 1 and indicates if the DMA channel is a + low priority (0) or high priority (1) channel.
What is purpose of adding sysfs entries here ?
+ +#define QOS_N_OFFSET 0x300 +#define CFG_OFFSET 0x400 +#define MAX_BUS_REQ_LEN_OFFSET 0x41C +#define MAX_XACTIONS_OFFSET 0x420 +#define HW_VERSION_OFFSET 0x424 +#define CHRESET_TIMEOUT_OFFSET 0x418 + +#define MAX_WR_XACTIONS_MASK GENMASK(4, 0) +#define MAX_RD_XACTIONS_MASK GENMASK(4, 0) +#define WEIGHT_MASK GENMASK(6, 0) +#define MAX_BUS_REQ_LEN_MASK GENMASK(15, 0) +#define CHRESET_TIMEOUUT_MASK GENMASK(19, 0) + +#define MAX_WR_XACTIONS_BIT_POS 16 +#define MAX_BUS_WR_REQ_BIT_POS 16 +#define WRR_BIT_POS 8 +#define PRIORITY_BIT_POS 15 + +#define AUTOSUSPEND_TIMEOUT 2000 +#define MAX_CHANNEL_WEIGHT 15
These names are quite generic and prone to collide with generic names, please prefix them with your driver name
+} +EXPORT_SYMBOL_GPL(hidma_mgmt_setup);
Why is this exported or rather who would be users of this?
+static int hidma_mgmt_probe(struct platform_device *pdev)
+{
+ struct hidma_mgmt_dev *mgmtdev;
+ struct resource *res;
+ void __iomem *virtaddr;
+ int irq;
+ int rc;
+ u32 val;
+
+ pm_runtime_set_autosuspend_delay(&pdev->dev, AUTOSUSPEND_TIMEOUT);
+ pm_runtime_use_autosuspend(&pdev->dev);
+ pm_runtime_set_active(&pdev->dev);
+ pm_runtime_enable(&pdev->dev);at this time pm core will treat device as fully enabled and pm methods can be invoked, but you are not ready yet right. Typically these are done at the end of the probe unless you have a reason...
+static ssize_t show_values(struct device *dev, struct device_attribute *attr, + char *buf)
Please fix the coding style here and other places as well. Specifically please read Chapter 2 -- ~Vinod