linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Add Support for Broadcom D1W controller
@ 2017-08-16 17:45 Scott Branden
  2017-08-16 17:45 ` [PATCH 1/3] dt-bindings: mfd: d1w: iproc: Documentation for d1w driver Scott Branden
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Scott Branden @ 2017-08-16 17:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: BCM Kernel Feedback, linux-arm-kernel, linux-kernel, Scott Branden

Add support for Broadcom Dallas One wire (D1W) bus master controller.

This patch series adds a callback for the ds1wm read/write functions
to the common DS1W driver code and the Broadcom specific DS1W controller.

Shreesha Rajashekar (3):
  dt-bindings: mfd: d1w: iproc: Documentation for d1w driver
  w1: d1w: Provide callback for ds1wm read/write
  mfd: d1w: iproc: Add d1w driver

 .../devicetree/bindings/mfd/brcm,iproc-d1w.txt     |  27 +++
 drivers/mfd/Kconfig                                |  11 ++
 drivers/mfd/Makefile                               |   1 +
 drivers/mfd/bcm-iproc-d1w.c                        | 202 +++++++++++++++++++++
 drivers/w1/masters/ds1wm.c                         |  18 +-
 include/linux/mfd/ds1wm.h                          |   2 +
 6 files changed, 259 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/brcm,iproc-d1w.txt
 create mode 100644 drivers/mfd/bcm-iproc-d1w.c

-- 
2.5.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] dt-bindings: mfd: d1w: iproc: Documentation for d1w driver
  2017-08-16 17:45 [PATCH 0/3] Add Support for Broadcom D1W controller Scott Branden
@ 2017-08-16 17:45 ` Scott Branden
  2017-08-16 20:45   ` Rob Herring
  2017-08-16 17:45 ` [PATCH 2/3] w1: d1w: Provide callback for ds1wm read/write Scott Branden
  2017-08-16 17:45 ` [PATCH 3/3] mfd: d1w: iproc: Add d1w driver Scott Branden
  2 siblings, 1 reply; 9+ messages in thread
From: Scott Branden @ 2017-08-16 17:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: BCM Kernel Feedback, linux-arm-kernel, linux-kernel,
	Shreesha Rajashekar, Scott Branden

From: Shreesha Rajashekar <shreesha@broadcom.com>

Adding document for IPROC d1w driver.

Signed-off-by: Shreesha Rajashekar <shreesha@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
---
 .../devicetree/bindings/mfd/brcm,iproc-d1w.txt     | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/brcm,iproc-d1w.txt

diff --git a/Documentation/devicetree/bindings/mfd/brcm,iproc-d1w.txt b/Documentation/devicetree/bindings/mfd/brcm,iproc-d1w.txt
new file mode 100644
index 0000000..2bb99c1
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/brcm,iproc-d1w.txt
@@ -0,0 +1,27 @@
+* Broadcom Dallas One wire (D1W) bus master controller
+
+Required properties:
+- compatible : should be "brcm,iproc-d1w"
+- reg : Address and length of the register set for the device
+- interrupts : IRQ number of D1W controller
+
+Optional properties:
+- clocks : phandle of clock that supplies the module (required if platform
+		clock bindings use device tree)
+- clock-names : name for the clock
+- clock-frequency : D1W divisor clock rate
+- reset-recover-delay : Delay while reset D1W in milliseconds.
+
+Example:
+
+- From bcm-cygnus.dtsi:
+iproc_d1w: d1w@180ab000 {
+	compatible = "brcm,iproc_d1w";
+	reg = <0x180ab000 0x0f>;
+	interrupts = <GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&axi81_clk>;
+	clock-names = "iproc_d1w_clk";
+	clock-frequency = <100000000>;
+	reset-recover-delay = <1>;
+};
+
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] w1: d1w: Provide callback for ds1wm read/write
  2017-08-16 17:45 [PATCH 0/3] Add Support for Broadcom D1W controller Scott Branden
  2017-08-16 17:45 ` [PATCH 1/3] dt-bindings: mfd: d1w: iproc: Documentation for d1w driver Scott Branden
@ 2017-08-16 17:45 ` Scott Branden
  2017-08-17  1:10   ` Florian Fainelli
  2017-08-16 17:45 ` [PATCH 3/3] mfd: d1w: iproc: Add d1w driver Scott Branden
  2 siblings, 1 reply; 9+ messages in thread
From: Scott Branden @ 2017-08-16 17:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: BCM Kernel Feedback, linux-arm-kernel, linux-kernel,
	Shreesha Rajashekar, Scott Branden

From: Shreesha Rajashekar <shreesha@broadcom.com>

DS1WM core registers are accessed by reading from and writing to a group of
registers in iproc SOC's.

By default the read and write function uses
__raw_readb() and __raw_writeb(), which wouldnt work for iproc.
hence modifying to provide callbacks for read and write functions.

Signed-off-by: Shreesha Rajashekar <shreesha@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
---
 drivers/w1/masters/ds1wm.c | 18 ++++++++++++++++--
 include/linux/mfd/ds1wm.h  |  2 ++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/masters/ds1wm.c b/drivers/w1/masters/ds1wm.c
index fd2e9da..9fadc39 100644
--- a/drivers/w1/masters/ds1wm.c
+++ b/drivers/w1/masters/ds1wm.c
@@ -115,12 +115,26 @@ struct ds1wm_data {
 static inline void ds1wm_write_register(struct ds1wm_data *ds1wm_data, u32 reg,
 					u8 val)
 {
-	__raw_writeb(val, ds1wm_data->map + (reg << ds1wm_data->bus_shift));
+	struct device *dev = &ds1wm_data->pdev->dev;
+	struct ds1wm_driver_data *pdata = dev_get_platdata(dev);
+
+	if (pdata->write)
+		pdata->write(ds1wm_data->map, reg, val);
+	else
+		__raw_writeb(val, ds1wm_data->map +
+			(reg << ds1wm_data->bus_shift));
 }
 
 static inline u8 ds1wm_read_register(struct ds1wm_data *ds1wm_data, u32 reg)
 {
-	return __raw_readb(ds1wm_data->map + (reg << ds1wm_data->bus_shift));
+	struct device *dev = &ds1wm_data->pdev->dev;
+	struct ds1wm_driver_data *pdata = dev_get_platdata(dev);
+
+	if (pdata->read)
+		return pdata->read(ds1wm_data->map, reg);
+	else
+		return __raw_readb(ds1wm_data->map +
+				(reg << ds1wm_data->bus_shift));
 }
 
 
diff --git a/include/linux/mfd/ds1wm.h b/include/linux/mfd/ds1wm.h
index 38a372a..c2d41d3 100644
--- a/include/linux/mfd/ds1wm.h
+++ b/include/linux/mfd/ds1wm.h
@@ -10,4 +10,6 @@ struct ds1wm_driver_data {
 	/* ds1wm implements the precise timings of*/
 	/* a reset pulse/presence detect sequence.*/
 	unsigned int reset_recover_delay;
+	void (*write)(void __iomem *map, u32 reg, u8 val);
+	u8 (*read)(void __iomem *map, u32 reg);
 };
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] mfd: d1w: iproc: Add d1w driver
  2017-08-16 17:45 [PATCH 0/3] Add Support for Broadcom D1W controller Scott Branden
  2017-08-16 17:45 ` [PATCH 1/3] dt-bindings: mfd: d1w: iproc: Documentation for d1w driver Scott Branden
  2017-08-16 17:45 ` [PATCH 2/3] w1: d1w: Provide callback for ds1wm read/write Scott Branden
@ 2017-08-16 17:45 ` Scott Branden
  2017-08-17 10:02   ` Lee Jones
  2 siblings, 1 reply; 9+ messages in thread
From: Scott Branden @ 2017-08-16 17:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: BCM Kernel Feedback, linux-arm-kernel, linux-kernel,
	Shreesha Rajashekar, Scott Branden

From: Shreesha Rajashekar <shreesha@broadcom.com>

d1w bus master controller is added as a mfd node.
ds1wm driver is hooked to this node through the mfd framework.

Signed-off-by: Shreesha Rajashekar <shreesha@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
---
 drivers/mfd/Kconfig         |  11 +++
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/bcm-iproc-d1w.c | 202 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 214 insertions(+)
 create mode 100644 drivers/mfd/bcm-iproc-d1w.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 94ad2c1..a7d9335 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -216,6 +216,17 @@ config MFD_ASIC3
 	  This driver supports the ASIC3 multifunction chip found on many
 	  PDAs (mainly iPAQ and HTC based ones)
 
+config MFD_IPROC_D1W
+	bool "IPROC DS1WM one wire interface"
+	depends on (ARCH_BCM_IPROC || COMPILE_TEST)
+	select MFD_CORE
+	default ARCH_BCM_IPROC
+	help
+	  This driver provides support to the Dallas One Wire (D1W).This block
+	  is found in various Broadcom iProc family of SoCs.
+	  The Cygnus Dallas 1-Wire Interface provides complete
+	  control of the 1-Wire bus through 8-bit commands.
+
 config PMIC_DA903X
 	bool "Dialog Semiconductor DA9030/DA9034 PMIC Support"
 	depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 080793b..2f39fdd 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -217,6 +217,7 @@ obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
 obj-$(CONFIG_INTEL_SOC_PMIC_BXTWC)	+= intel_soc_pmic_bxtwc.o
 obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC)	+= intel_soc_pmic_chtwc.o
 obj-$(CONFIG_MFD_MT6397)	+= mt6397-core.o
+obj-$(CONFIG_MFD_IPROC_D1W)	+= bcm-iproc-d1w.o
 
 obj-$(CONFIG_MFD_ALTERA_A10SR)	+= altera-a10sr.o
 obj-$(CONFIG_MFD_SUN4I_GPADC)	+= sun4i-gpadc.o
diff --git a/drivers/mfd/bcm-iproc-d1w.c b/drivers/mfd/bcm-iproc-d1w.c
new file mode 100644
index 0000000..d439060
--- /dev/null
+++ b/drivers/mfd/bcm-iproc-d1w.c
@@ -0,0 +1,202 @@
+/*
+ * Copyright (C) 2016 Broadcom.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/stddef.h>
+#include <linux/of.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/ds1wm.h>
+
+#define D1W_DIN_OFFSET  0x00
+#define D1W_DOUT_OFFSET 0x04
+#define D1W_ADDR_OFFSET 0x08
+#define D1W_CTRL_OFFSET 0x0C
+
+#define WRITE_CMD 0x3
+#define READ_CMD 0x2
+
+#define DEFAULT_RESET_RECOVERY_DELAY	1
+/* CLOCK_RATE programmed in the IP divisor register DS1WM_CLKDIV*/
+#define DEFAULT_CLOCK_RATE		100000000
+
+struct iproc_ds1wm_core {
+	struct ds1wm_driver_data plat_data;
+	struct clk *ds1wm_clk;
+};
+
+static void iproc_d1w_write_register(void __iomem *base, u32 reg,
+		u8 val)
+{
+	writel(val, base + D1W_DIN_OFFSET);
+	writel(reg, base + D1W_ADDR_OFFSET);
+	writel(WRITE_CMD, base + D1W_CTRL_OFFSET);
+}
+
+static u8 iproc_d1w_read_register(void __iomem *base, u32 reg)
+{
+	writel(reg, base + D1W_ADDR_OFFSET);
+	writel(READ_CMD, base + D1W_CTRL_OFFSET);
+	return readb(base + D1W_DOUT_OFFSET);
+}
+
+static int iproc_ds1wm_enable(struct platform_device *pdev)
+{
+	struct iproc_ds1wm_core *ds1wm_core;
+
+	ds1wm_core = dev_get_drvdata(pdev->dev.parent);
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+
+	if (ds1wm_core->ds1wm_clk)
+		return clk_prepare_enable(ds1wm_core->ds1wm_clk);
+
+	return 0;
+}
+
+static int iproc_ds1wm_disable(struct platform_device *pdev)
+{
+	struct iproc_ds1wm_core *ds1wm_core;
+
+	ds1wm_core = dev_get_drvdata(pdev->dev.parent);
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+	if (ds1wm_core->ds1wm_clk)
+		clk_disable_unprepare(ds1wm_core->ds1wm_clk);
+
+	return 0;
+}
+
+static struct resource iproc_ds1wm_resources[] = {
+	[0] = {
+		.flags = IORESOURCE_MEM,
+	},
+	[1] = {
+		.flags = IORESOURCE_IRQ,
+	}
+};
+
+static struct mfd_cell iproc_ds1wm_mfd_cell = {
+	.name          = "ds1wm",
+	.enable        = iproc_ds1wm_enable,
+	.disable       = iproc_ds1wm_disable,
+	.num_resources = ARRAY_SIZE(iproc_ds1wm_resources),
+	.resources     = iproc_ds1wm_resources,
+};
+
+static int iproc_d1w_dt_binding(struct platform_device *pdev,
+		struct iproc_ds1wm_core *ds1wm_core)
+{
+	int ret;
+
+	ret = of_property_read_u32(pdev->dev.of_node,
+			"reset-recover-delay",
+			&ds1wm_core->plat_data.reset_recover_delay);
+	if (ret < 0)
+		ds1wm_core->plat_data.reset_recover_delay =
+				DEFAULT_RESET_RECOVERY_DELAY;
+
+	ret = of_property_read_u32(pdev->dev.of_node,
+			"clock-frequency", &ds1wm_core->plat_data.clock_rate);
+	if (ret < 0)
+		ds1wm_core->plat_data.clock_rate = DEFAULT_CLOCK_RATE;
+
+	ds1wm_core->ds1wm_clk = devm_clk_get(&pdev->dev, "iproc_d1w_clk");
+	if (IS_ERR(ds1wm_core->ds1wm_clk)) {
+		dev_info(&pdev->dev,
+				"No clock specified. Assuming it's enabled\n");
+		ds1wm_core->ds1wm_clk = NULL;
+	}
+	ret = clk_set_rate(ds1wm_core->ds1wm_clk,
+		ds1wm_core->plat_data.clock_rate);
+	if (ret)
+		dev_info(&pdev->dev,
+		"Failed to set to %u\n", ds1wm_core->plat_data.clock_rate);
+	return ret;
+}
+
+static int iproc_d1w_mfd_init(struct platform_device *pdev)
+{
+	struct resource *res;
+	int ret;
+	int irq;
+	struct iproc_ds1wm_core *ds1wm_core;
+
+	ds1wm_core = platform_get_drvdata(pdev);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "No Memory Resource specified\n");
+		return -ENODEV;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "No IRQ specified\n");
+		return irq;
+	}
+
+	ds1wm_core->plat_data.read = iproc_d1w_read_register;
+	ds1wm_core->plat_data.write = iproc_d1w_write_register;
+
+	ret = mfd_add_devices(&pdev->dev, pdev->id,
+			      &iproc_ds1wm_mfd_cell, 1, res, irq, NULL);
+	if (ret < 0)
+		dev_err(&pdev->dev, "failed to register iproc_d1w mfd\n");
+
+	dev_dbg(&pdev->dev, "Added iproc_d1w mfd successfully\n");
+	return ret;
+}
+
+static int iproc_d1w_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct iproc_ds1wm_core *ds1wm_core;
+
+	ds1wm_core = devm_kzalloc(&pdev->dev,
+			sizeof(struct iproc_ds1wm_core), GFP_KERNEL);
+	if (!ds1wm_core)
+		return -ENOMEM;
+
+	ret = iproc_d1w_dt_binding(pdev, ds1wm_core);
+	if (ret) {
+		dev_err(&pdev->dev, "Probe failed\n");
+		return ret;
+	}
+
+	iproc_ds1wm_mfd_cell.platform_data = &ds1wm_core->plat_data;
+	iproc_ds1wm_mfd_cell.pdata_size = sizeof(struct ds1wm_driver_data);
+
+	platform_set_drvdata(pdev, ds1wm_core);
+	ret = iproc_d1w_mfd_init(pdev);
+
+	return ret;
+}
+
+static const struct of_device_id iproc_d1w_of_match[] = {
+	{ .compatible = "brcm,iproc-d1w" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, iproc_d1w_of_match);
+
+static struct platform_driver iproc_d1w_driver = {
+	.driver = {
+		.name = "brcm,iproc-d1w",
+		.of_match_table = of_match_ptr(iproc_d1w_of_match),
+	},
+	.probe = iproc_d1w_probe,
+};
+module_platform_driver(iproc_d1w_driver);
+
+MODULE_AUTHOR("Broadcom");
+MODULE_DESCRIPTION("Broadcom one wire busmaster Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] dt-bindings: mfd: d1w: iproc: Documentation for d1w driver
  2017-08-16 17:45 ` [PATCH 1/3] dt-bindings: mfd: d1w: iproc: Documentation for d1w driver Scott Branden
@ 2017-08-16 20:45   ` Rob Herring
  2017-08-17 18:20     ` Scott Branden
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2017-08-16 20:45 UTC (permalink / raw)
  To: Scott Branden
  Cc: Greg Kroah-Hartman, BCM Kernel Feedback, linux-arm-kernel,
	Linux Kernel Mailing List, Shreesha Rajashekar

On Wed, Aug 16, 2017 at 12:45 PM, Scott Branden
<scott.branden@broadcom.com> wrote:
> From: Shreesha Rajashekar <shreesha@broadcom.com>
>
> Adding document for IPROC d1w driver.
>
> Signed-off-by: Shreesha Rajashekar <shreesha@broadcom.com>
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
> ---
>  .../devicetree/bindings/mfd/brcm,iproc-d1w.txt     | 27 ++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/brcm,iproc-d1w.txt

How is this an MFD? Should be bindings/w1/...

>
> diff --git a/Documentation/devicetree/bindings/mfd/brcm,iproc-d1w.txt b/Documentation/devicetree/bindings/mfd/brcm,iproc-d1w.txt
> new file mode 100644
> index 0000000..2bb99c1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/brcm,iproc-d1w.txt
> @@ -0,0 +1,27 @@
> +* Broadcom Dallas One wire (D1W) bus master controller
> +
> +Required properties:
> +- compatible : should be "brcm,iproc-d1w"
> +- reg : Address and length of the register set for the device
> +- interrupts : IRQ number of D1W controller
> +
> +Optional properties:
> +- clocks : phandle of clock that supplies the module (required if platform
> +               clock bindings use device tree)
> +- clock-names : name for the clock
> +- clock-frequency : D1W divisor clock rate

This should be bus-frequency. We messed up on I2C.

> +- reset-recover-delay : Delay while reset D1W in milliseconds.

Is this a standard 1-wire bus operation? I'm guessing so, but if not
needs a vendor prefix.

Also, needs unit suffix as defined in property-units.txt.

> +
> +Example:
> +
> +- From bcm-cygnus.dtsi:
> +iproc_d1w: d1w@180ab000 {

Should be a generic name. So far in bindings/w1/ we have 3 different
ones in 3 bindings. Lets go with "onewire".

> +       compatible = "brcm,iproc_d1w";

Doesn't match the doc.

> +       reg = <0x180ab000 0x0f>;
> +       interrupts = <GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>;
> +       clocks = <&axi81_clk>;
> +       clock-names = "iproc_d1w_clk";
> +       clock-frequency = <100000000>;
> +       reset-recover-delay = <1>;
> +};
> +
> --
> 2.5.0
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] w1: d1w: Provide callback for ds1wm read/write
  2017-08-16 17:45 ` [PATCH 2/3] w1: d1w: Provide callback for ds1wm read/write Scott Branden
@ 2017-08-17  1:10   ` Florian Fainelli
  0 siblings, 0 replies; 9+ messages in thread
From: Florian Fainelli @ 2017-08-17  1:10 UTC (permalink / raw)
  To: Scott Branden, Greg Kroah-Hartman
  Cc: BCM Kernel Feedback, linux-arm-kernel, linux-kernel, Shreesha Rajashekar



On 08/16/2017 10:45 AM, Scott Branden wrote:
> From: Shreesha Rajashekar <shreesha@broadcom.com>
> 
> DS1WM core registers are accessed by reading from and writing to a group of
> registers in iproc SOC's.
> 
> By default the read and write function uses
> __raw_readb() and __raw_writeb(), which wouldnt work for iproc.
> hence modifying to provide callbacks for read and write functions.

It's only obvious once you look at patch 3, and that's because you use
MFD, might be worth adding to this commit message here.

> 
> Signed-off-by: Shreesha Rajashekar <shreesha@broadcom.com>
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
> ---
>  drivers/w1/masters/ds1wm.c | 18 ++++++++++++++++--
>  include/linux/mfd/ds1wm.h  |  2 ++
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/w1/masters/ds1wm.c b/drivers/w1/masters/ds1wm.c
> index fd2e9da..9fadc39 100644
> --- a/drivers/w1/masters/ds1wm.c
> +++ b/drivers/w1/masters/ds1wm.c
> @@ -115,12 +115,26 @@ struct ds1wm_data {
>  static inline void ds1wm_write_register(struct ds1wm_data *ds1wm_data, u32 reg,
>  					u8 val)
>  {
> -	__raw_writeb(val, ds1wm_data->map + (reg << ds1wm_data->bus_shift));
> +	struct device *dev = &ds1wm_data->pdev->dev;
> +	struct ds1wm_driver_data *pdata = dev_get_platdata(dev);
> +
> +	if (pdata->write)
> +		pdata->write(ds1wm_data->map, reg, val);

Should not this be pdata && pdata->write otherwise are not you just
causing a NULL pointer dereference for any driver other than the
Broadcom iProc d1w?

Also, you may not have any bus shift, but it sounds like this should
either be clarified in a header file that the platform data I/O accessor
is responsible for shifting, or that the shift should be applied here
(and because you set it to 0, nothing happens).
-- 
Florian

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] mfd: d1w: iproc: Add d1w driver
  2017-08-16 17:45 ` [PATCH 3/3] mfd: d1w: iproc: Add d1w driver Scott Branden
@ 2017-08-17 10:02   ` Lee Jones
  2017-08-17 18:19     ` Scott Branden
  0 siblings, 1 reply; 9+ messages in thread
From: Lee Jones @ 2017-08-17 10:02 UTC (permalink / raw)
  To: Scott Branden
  Cc: Greg Kroah-Hartman, BCM Kernel Feedback, linux-arm-kernel,
	linux-kernel, Shreesha Rajashekar

On Wed, 16 Aug 2017, Scott Branden wrote:

> From: Shreesha Rajashekar <shreesha@broadcom.com>
> 
> d1w bus master controller is added as a mfd node.

Why?

> ds1wm driver is hooked to this node through the mfd framework.

Why?

What makes this an MFD device?

> Signed-off-by: Shreesha Rajashekar <shreesha@broadcom.com>
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
> ---
>  drivers/mfd/Kconfig         |  11 +++
>  drivers/mfd/Makefile        |   1 +
>  drivers/mfd/bcm-iproc-d1w.c | 202 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 214 insertions(+)
>  create mode 100644 drivers/mfd/bcm-iproc-d1w.c

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] mfd: d1w: iproc: Add d1w driver
  2017-08-17 10:02   ` Lee Jones
@ 2017-08-17 18:19     ` Scott Branden
  0 siblings, 0 replies; 9+ messages in thread
From: Scott Branden @ 2017-08-17 18:19 UTC (permalink / raw)
  To: Lee Jones
  Cc: Greg Kroah-Hartman, BCM Kernel Feedback, linux-arm-kernel,
	linux-kernel, Shreesha Rajashekar

Hi Lee,


On 17-08-17 03:02 AM, Lee Jones wrote:
> On Wed, 16 Aug 2017, Scott Branden wrote:
>
>> From: Shreesha Rajashekar <shreesha@broadcom.com>
>>
>> d1w bus master controller is added as a mfd node.
> Why?
>
>> ds1wm driver is hooked to this node through the mfd framework.
> Why?
It looks like it was added under mfd as htc-pasic3.c is there
(other than htc-pasic3.c is and mfd due to gpio nodes).

Yes, bcm-iproc-d1w should move under w1 directory.
>
> What makes this an MFD device?
>
>> Signed-off-by: Shreesha Rajashekar <shreesha@broadcom.com>
>> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
>> ---
>>   drivers/mfd/Kconfig         |  11 +++
>>   drivers/mfd/Makefile        |   1 +
>>   drivers/mfd/bcm-iproc-d1w.c | 202 ++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 214 insertions(+)
>>   create mode 100644 drivers/mfd/bcm-iproc-d1w.c

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] dt-bindings: mfd: d1w: iproc: Documentation for d1w driver
  2017-08-16 20:45   ` Rob Herring
@ 2017-08-17 18:20     ` Scott Branden
  0 siblings, 0 replies; 9+ messages in thread
From: Scott Branden @ 2017-08-17 18:20 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, BCM Kernel Feedback, linux-arm-kernel,
	Linux Kernel Mailing List, Shreesha Rajashekar

Thanks for review Rob


On 17-08-16 01:45 PM, Rob Herring wrote:
> On Wed, Aug 16, 2017 at 12:45 PM, Scott Branden
> <scott.branden@broadcom.com> wrote:
>> From: Shreesha Rajashekar <shreesha@broadcom.com>
>>
>> Adding document for IPROC d1w driver.
>>
>> Signed-off-by: Shreesha Rajashekar <shreesha@broadcom.com>
>> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
>> ---
>>   .../devicetree/bindings/mfd/brcm,iproc-d1w.txt     | 27 ++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/mfd/brcm,iproc-d1w.txt
> How is this an MFD? Should be bindings/w1/...
>
>> diff --git a/Documentation/devicetree/bindings/mfd/brcm,iproc-d1w.txt b/Documentation/devicetree/bindings/mfd/brcm,iproc-d1w.txt
>> new file mode 100644
>> index 0000000..2bb99c1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/brcm,iproc-d1w.txt
>> @@ -0,0 +1,27 @@
>> +* Broadcom Dallas One wire (D1W) bus master controller
>> +
>> +Required properties:
>> +- compatible : should be "brcm,iproc-d1w"
>> +- reg : Address and length of the register set for the device
>> +- interrupts : IRQ number of D1W controller
>> +
>> +Optional properties:
>> +- clocks : phandle of clock that supplies the module (required if platform
>> +               clock bindings use device tree)
>> +- clock-names : name for the clock
>> +- clock-frequency : D1W divisor clock rate
> This should be bus-frequency. We messed up on I2C.
>
>> +- reset-recover-delay : Delay while reset D1W in milliseconds.
> Is this a standard 1-wire bus operation? I'm guessing so, but if not
> needs a vendor prefix.
>
> Also, needs unit suffix as defined in property-units.txt.
>
>> +
>> +Example:
>> +
>> +- From bcm-cygnus.dtsi:
>> +iproc_d1w: d1w@180ab000 {
> Should be a generic name. So far in bindings/w1/ we have 3 different
> ones in 3 bindings. Lets go with "onewire".
>
>> +       compatible = "brcm,iproc_d1w";
> Doesn't match the doc.
We have some cleanup work to do.  Need to move to mfd and address your 
comments.
>
>> +       reg = <0x180ab000 0x0f>;
>> +       interrupts = <GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>;
>> +       clocks = <&axi81_clk>;
>> +       clock-names = "iproc_d1w_clk";
>> +       clock-frequency = <100000000>;
>> +       reset-recover-delay = <1>;
>> +};
>> +
>> --
>> 2.5.0
>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-08-17 18:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-16 17:45 [PATCH 0/3] Add Support for Broadcom D1W controller Scott Branden
2017-08-16 17:45 ` [PATCH 1/3] dt-bindings: mfd: d1w: iproc: Documentation for d1w driver Scott Branden
2017-08-16 20:45   ` Rob Herring
2017-08-17 18:20     ` Scott Branden
2017-08-16 17:45 ` [PATCH 2/3] w1: d1w: Provide callback for ds1wm read/write Scott Branden
2017-08-17  1:10   ` Florian Fainelli
2017-08-16 17:45 ` [PATCH 3/3] mfd: d1w: iproc: Add d1w driver Scott Branden
2017-08-17 10:02   ` Lee Jones
2017-08-17 18:19     ` Scott Branden

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).