Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932456AbbEMH1X (ORCPT ); Wed, 13 May 2015 03:27:23 -0400 Received: from arroyo.ext.ti.com ([192.94.94.40]:43951 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752273AbbEMH1V (ORCPT ); Wed, 13 May 2015 03:27:21 -0400 Message-ID: <5552FCB2.8060300@ti.com> Date: Wed, 13 May 2015 12:56:42 +0530 From: Vignesh R User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Evgeniy Polyakov , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Jonathan Corbet CC: Andrew Morton , NeilBrown , Tony Lindgren , Greg Kroah-Hartman , Fabian Frederick , , , Subject: Re: [PATCH RESEND] w1: masters: omap_hdq: Add support for 1-wire mode References: <1430728735-22747-1-git-send-email-vigneshr@ti.com> In-Reply-To: <1430728735-22747-1-git-send-email-vigneshr@ti.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 15697 Lines: 470 On Monday 04 May 2015 02:08 PM, Vignesh R wrote: > This patches makes following changes to omap_hdq driver > - Enable 1-wire mode. > - Implement w1_triplet callback to facilitate search rom > procedure and auto detection of 1-wire slaves. > - Proper enabling and disabling of interrupt. > - Cleanups (formatting and return value checks). > > HDQ mode remains unchanged. > > Signed-off-by: Vignesh R > --- > > This patch was sent for v4.1 but was never picked up. > Here is the link to the previous patch > http://www.spinics.net/lists/linux-omap/msg116228.html > Hi Andrew, Evgeniy Polyakov, Gentle ping.... This patch has been waiting since 4.1 merge window. Regards Vignesh > Documentation/devicetree/bindings/w1/omap-hdq.txt | 7 +- > Documentation/w1/masters/omap-hdq | 6 + > drivers/w1/masters/omap_hdq.c | 209 ++++++++++++++++++---- > 3 files changed, 188 insertions(+), 34 deletions(-) > > diff --git a/Documentation/devicetree/bindings/w1/omap-hdq.txt b/Documentation/devicetree/bindings/w1/omap-hdq.txt > index fef794741bd1..913c5f91a0f9 100644 > --- a/Documentation/devicetree/bindings/w1/omap-hdq.txt > +++ b/Documentation/devicetree/bindings/w1/omap-hdq.txt > @@ -1,11 +1,15 @@ > * OMAP HDQ One wire bus master controller > > Required properties: > -- compatible : should be "ti,omap3-1w" > +- compatible : should be "ti,omap3-1w" or "ti,am4372-hdq" > - reg : Address and length of the register set for the device > - interrupts : interrupt line. > - ti,hwmods : "hdq1w" > > +Optional properties: > +- ti,mode: should be "hdq": HDQ mode "1w": one-wire mode. > + If not specified HDQ mode is implied. > + > Example: > > - From omap3.dtsi > @@ -14,4 +18,5 @@ Example: > reg = <0x480b2000 0x1000>; > interrupts = <58>; > ti,hwmods = "hdq1w"; > + ti,mode = "hdq"; > }; > diff --git a/Documentation/w1/masters/omap-hdq b/Documentation/w1/masters/omap-hdq > index 884dc284b215..234522709a5f 100644 > --- a/Documentation/w1/masters/omap-hdq > +++ b/Documentation/w1/masters/omap-hdq > @@ -44,3 +44,9 @@ e.g: > insmod omap_hdq.ko W1_ID=2 > inamod w1_bq27000.ko F_ID=2 > > +The driver also supports 1-wire mode. In this mode, there is no need to > +pass slave ID as parameter. The driver will auto-detect slaves connected > +to the bus using SEARCH_ROM procedure. 1-wire mode can be selected by > +setting "ti,mode" property to "1w" in DT (see > +Documentation/devicetree/bindings/w1/omap-hdq.txt for more details). > +By default driver is in HDQ mode. > diff --git a/drivers/w1/masters/omap_hdq.c b/drivers/w1/masters/omap_hdq.c > index e7d448963a24..98aaade59651 100644 > --- a/drivers/w1/masters/omap_hdq.c > +++ b/drivers/w1/masters/omap_hdq.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > > #include "../w1.h" > #include "../w1_int.h" > @@ -27,21 +28,23 @@ > #define OMAP_HDQ_TX_DATA 0x04 > #define OMAP_HDQ_RX_DATA 0x08 > #define OMAP_HDQ_CTRL_STATUS 0x0c > -#define OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK (1<<6) > -#define OMAP_HDQ_CTRL_STATUS_CLOCKENABLE (1<<5) > -#define OMAP_HDQ_CTRL_STATUS_GO (1<<4) > -#define OMAP_HDQ_CTRL_STATUS_INITIALIZATION (1<<2) > -#define OMAP_HDQ_CTRL_STATUS_DIR (1<<1) > -#define OMAP_HDQ_CTRL_STATUS_MODE (1<<0) > +#define OMAP_HDQ_CTRL_STATUS_SINGLE BIT(7) > +#define OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK BIT(6) > +#define OMAP_HDQ_CTRL_STATUS_CLOCKENABLE BIT(5) > +#define OMAP_HDQ_CTRL_STATUS_GO BIT(4) > +#define OMAP_HDQ_CTRL_STATUS_PRESENCE BIT(3) > +#define OMAP_HDQ_CTRL_STATUS_INITIALIZATION BIT(2) > +#define OMAP_HDQ_CTRL_STATUS_DIR BIT(1) > #define OMAP_HDQ_INT_STATUS 0x10 > -#define OMAP_HDQ_INT_STATUS_TXCOMPLETE (1<<2) > -#define OMAP_HDQ_INT_STATUS_RXCOMPLETE (1<<1) > -#define OMAP_HDQ_INT_STATUS_TIMEOUT (1<<0) > +#define OMAP_HDQ_INT_STATUS_TXCOMPLETE BIT(2) > +#define OMAP_HDQ_INT_STATUS_RXCOMPLETE BIT(1) > +#define OMAP_HDQ_INT_STATUS_TIMEOUT BIT(0) > #define OMAP_HDQ_SYSCONFIG 0x14 > -#define OMAP_HDQ_SYSCONFIG_SOFTRESET (1<<1) > -#define OMAP_HDQ_SYSCONFIG_AUTOIDLE (1<<0) > +#define OMAP_HDQ_SYSCONFIG_SOFTRESET BIT(1) > +#define OMAP_HDQ_SYSCONFIG_AUTOIDLE BIT(0) > +#define OMAP_HDQ_SYSCONFIG_NOIDLE BIT(0) > #define OMAP_HDQ_SYSSTATUS 0x18 > -#define OMAP_HDQ_SYSSTATUS_RESETDONE (1<<0) > +#define OMAP_HDQ_SYSSTATUS_RESETDONE BIT(0) > > #define OMAP_HDQ_FLAG_CLEAR 0 > #define OMAP_HDQ_FLAG_SET 1 > @@ -67,6 +70,10 @@ struct hdq_data { > * the data wrire or read. > */ > int init_trans; > + int rrw; > + /* mode: 0-HDQ 1-W1 */ > + int mode; > + > }; > > static int omap_hdq_probe(struct platform_device *pdev); > @@ -74,6 +81,7 @@ static int omap_hdq_remove(struct platform_device *pdev); > > static const struct of_device_id omap_hdq_dt_ids[] = { > { .compatible = "ti,omap3-1w" }, > + { .compatible = "ti,am4372-hdq" }, > {} > }; > MODULE_DEVICE_TABLE(of, omap_hdq_dt_ids); > @@ -90,15 +98,12 @@ static struct platform_driver omap_hdq_driver = { > static u8 omap_w1_read_byte(void *_hdq); > static void omap_w1_write_byte(void *_hdq, u8 byte); > static u8 omap_w1_reset_bus(void *_hdq); > -static void omap_w1_search_bus(void *_hdq, struct w1_master *master_dev, > - u8 search_type, w1_slave_found_callback slave_found); > > > static struct w1_bus_master omap_w1_master = { > .read_byte = omap_w1_read_byte, > .write_byte = omap_w1_write_byte, > .reset_bus = omap_w1_reset_bus, > - .search = omap_w1_search_bus, > }; > > /* HDQ register I/O routines */ > @@ -122,6 +127,15 @@ static inline u8 hdq_reg_merge(struct hdq_data *hdq_data, u32 offset, > return new_val; > } > > +static void hdq_disable_interrupt(struct hdq_data *hdq_data, u32 offset, > + u32 mask) > +{ > + u32 ie; > + > + ie = readl(hdq_data->hdq_base + offset); > + writel(ie & mask, hdq_data->hdq_base + offset); > +} > + > /* > * Wait for one or more bits in flag change. > * HDQ_FLAG_SET: wait until any bit in the flag is set. > @@ -229,13 +243,7 @@ static irqreturn_t hdq_isr(int irq, void *_hdq) > return IRQ_HANDLED; > } > > -/* HDQ Mode: always return success */ > -static u8 omap_w1_reset_bus(void *_hdq) > -{ > - return 0; > -} > - > -/* W1 search callback function */ > +/* W1 search callback function in HDQ mode */ > static void omap_w1_search_bus(void *_hdq, struct w1_master *master_dev, > u8 search_type, w1_slave_found_callback slave_found) > { > @@ -262,9 +270,10 @@ static int _omap_hdq_reset(struct hdq_data *hdq_data) > int ret; > u8 tmp_status; > > - hdq_reg_out(hdq_data, OMAP_HDQ_SYSCONFIG, OMAP_HDQ_SYSCONFIG_SOFTRESET); > + hdq_reg_out(hdq_data, OMAP_HDQ_SYSCONFIG, > + OMAP_HDQ_SYSCONFIG_SOFTRESET); > /* > - * Select HDQ mode & enable clocks. > + * Select HDQ/1W mode & enable clocks. > * It is observed that INT flags can't be cleared via a read and GO/INIT > * won't return to zero if interrupt is disabled. So we always enable > * interrupt. > @@ -282,7 +291,8 @@ static int _omap_hdq_reset(struct hdq_data *hdq_data) > else { > hdq_reg_out(hdq_data, OMAP_HDQ_CTRL_STATUS, > OMAP_HDQ_CTRL_STATUS_CLOCKENABLE | > - OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK); > + OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK | > + hdq_data->mode); > hdq_reg_out(hdq_data, OMAP_HDQ_SYSCONFIG, > OMAP_HDQ_SYSCONFIG_AUTOIDLE); > } > @@ -334,6 +344,18 @@ static int omap_hdq_break(struct hdq_data *hdq_data) > ret = -ETIMEDOUT; > goto out; > } > + > + /* > + * check for the presence detect bit to get > + * set to show that the slave is responding > + */ > + if (!(hdq_reg_in(hdq_data, OMAP_HDQ_CTRL_STATUS) & > + OMAP_HDQ_CTRL_STATUS_PRESENCE)) { > + dev_dbg(hdq_data->dev, "Presence bit not set\n"); > + ret = -ETIMEDOUT; > + goto out; > + } > + > /* > * wait for both INIT and GO bits rerurn to zero. > * zero wait time expected for interrupt mode. > @@ -368,6 +390,8 @@ static int hdq_read_byte(struct hdq_data *hdq_data, u8 *val) > goto out; > } > > + hdq_data->hdq_irqstatus = 0; > + > if (!(hdq_data->hdq_irqstatus & OMAP_HDQ_INT_STATUS_RXCOMPLETE)) { > hdq_reg_merge(hdq_data, OMAP_HDQ_CTRL_STATUS, > OMAP_HDQ_CTRL_STATUS_DIR | OMAP_HDQ_CTRL_STATUS_GO, > @@ -400,7 +424,7 @@ rtn: > > } > > -/* Enable clocks and set the controller to HDQ mode */ > +/* Enable clocks and set the controller to HDQ/1W mode */ > static int omap_hdq_get(struct hdq_data *hdq_data) > { > int ret = 0; > @@ -422,7 +446,7 @@ static int omap_hdq_get(struct hdq_data *hdq_data) > > pm_runtime_get_sync(hdq_data->dev); > > - /* make sure HDQ is out of reset */ > + /* make sure HDQ/1W is out of reset */ > if (!(hdq_reg_in(hdq_data, OMAP_HDQ_SYSSTATUS) & > OMAP_HDQ_SYSSTATUS_RESETDONE)) { > ret = _omap_hdq_reset(hdq_data); > @@ -430,12 +454,13 @@ static int omap_hdq_get(struct hdq_data *hdq_data) > /* back up the count */ > hdq_data->hdq_usecount--; > } else { > - /* select HDQ mode & enable clocks */ > + /* select HDQ/1W mode & enable clocks */ > hdq_reg_out(hdq_data, OMAP_HDQ_CTRL_STATUS, > OMAP_HDQ_CTRL_STATUS_CLOCKENABLE | > - OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK); > + OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK | > + hdq_data->mode); > hdq_reg_out(hdq_data, OMAP_HDQ_SYSCONFIG, > - OMAP_HDQ_SYSCONFIG_AUTOIDLE); > + OMAP_HDQ_SYSCONFIG_NOIDLE); > hdq_reg_in(hdq_data, OMAP_HDQ_INT_STATUS); > } > } > @@ -456,6 +481,8 @@ static int omap_hdq_put(struct hdq_data *hdq_data) > if (ret < 0) > return -EINTR; > > + hdq_reg_out(hdq_data, OMAP_HDQ_SYSCONFIG, > + OMAP_HDQ_SYSCONFIG_AUTOIDLE); > if (0 == hdq_data->hdq_usecount) { > dev_dbg(hdq_data->dev, "attempt to decrement use count" > " when it is zero"); > @@ -471,6 +498,85 @@ static int omap_hdq_put(struct hdq_data *hdq_data) > return ret; > } > > +/* > + * W1 triplet callback function - used for searching ROM addresses. > + * Registered only when controller is in 1-wire mode. > + */ > +static u8 omap_w1_triplet(void *_hdq, u8 bdir) > +{ > + u8 ret, id_bit, comp_bit; > + struct hdq_data *hdq_data = _hdq; > + u8 ctrl = OMAP_HDQ_CTRL_STATUS_SINGLE | OMAP_HDQ_CTRL_STATUS_GO | > + OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK; > + u8 mask = ctrl | OMAP_HDQ_CTRL_STATUS_DIR; > + > + omap_hdq_get(_hdq); > + > + ret = mutex_lock_interruptible(&hdq_data->hdq_mutex); > + if (ret < 0) { > + ret = -EINTR; > + goto rtn; > + } > + > + hdq_data->hdq_irqstatus = 0; > + /* read id_bit */ > + hdq_reg_merge(_hdq, OMAP_HDQ_CTRL_STATUS, > + ctrl | OMAP_HDQ_CTRL_STATUS_DIR, mask); > + wait_event_timeout(hdq_wait_queue, > + (hdq_data->hdq_irqstatus > + & OMAP_HDQ_INT_STATUS_RXCOMPLETE), > + OMAP_HDQ_TIMEOUT); > + id_bit = (hdq_reg_in(_hdq, OMAP_HDQ_RX_DATA) & 0x01); > + > + hdq_data->hdq_irqstatus = 0; > + /* read comp_bit */ > + hdq_reg_merge(_hdq, OMAP_HDQ_CTRL_STATUS, > + ctrl | OMAP_HDQ_CTRL_STATUS_DIR, mask); > + wait_event_timeout(hdq_wait_queue, > + (hdq_data->hdq_irqstatus > + & OMAP_HDQ_INT_STATUS_RXCOMPLETE), > + OMAP_HDQ_TIMEOUT); > + comp_bit = (hdq_reg_in(_hdq, OMAP_HDQ_RX_DATA) & 0x01); > + > + if (id_bit && comp_bit) { > + ret = 0x03; /* error */ > + goto rtn; > + } > + if (!id_bit && !comp_bit) { > + /* Both bits are valid, take the direction given */ > + ret = bdir ? 0x04 : 0; > + } else { > + /* Only one bit is valid, take that direction */ > + bdir = id_bit; > + ret = id_bit ? 0x05 : 0x02; > + } > + > + /* write bdir bit */ > + hdq_reg_out(_hdq, OMAP_HDQ_TX_DATA, bdir); > + hdq_reg_merge(_hdq, OMAP_HDQ_CTRL_STATUS, ctrl, mask); > + wait_event_timeout(hdq_wait_queue, > + (hdq_data->hdq_irqstatus > + & OMAP_HDQ_INT_STATUS_TXCOMPLETE), > + OMAP_HDQ_TIMEOUT); > + > + hdq_reg_merge(_hdq, OMAP_HDQ_CTRL_STATUS, 0, > + OMAP_HDQ_CTRL_STATUS_SINGLE); > + > +rtn: > + mutex_unlock(&hdq_data->hdq_mutex); > + omap_hdq_put(_hdq); > + return ret; > +} > + > +/* reset callback */ > +static u8 omap_w1_reset_bus(void *_hdq) > +{ > + omap_hdq_get(_hdq); > + omap_hdq_break(_hdq); > + omap_hdq_put(_hdq); > + return 0; > +} > + > /* Read a byte of data from the device */ > static u8 omap_w1_read_byte(void *_hdq) > { > @@ -478,6 +584,10 @@ static u8 omap_w1_read_byte(void *_hdq) > u8 val = 0; > int ret; > > + /* First write to initialize the transfer */ > + if (hdq_data->init_trans == 0) > + omap_hdq_get(hdq_data); > + > ret = hdq_read_byte(hdq_data, &val); > if (ret) { > ret = mutex_lock_interruptible(&hdq_data->hdq_mutex); > @@ -491,6 +601,10 @@ static u8 omap_w1_read_byte(void *_hdq) > return -1; > } > > + hdq_disable_interrupt(hdq_data, OMAP_HDQ_CTRL_STATUS, > + ~OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK); > + hdq_data->hdq_usecount = 0; > + > /* Write followed by a read, release the module */ > if (hdq_data->init_trans) { > ret = mutex_lock_interruptible(&hdq_data->hdq_mutex); > @@ -517,6 +631,14 @@ static void omap_w1_write_byte(void *_hdq, u8 byte) > if (hdq_data->init_trans == 0) > omap_hdq_get(hdq_data); > > + /* > + * We need to reset the slave before > + * issuing the SKIP ROM command, else > + * the slave will not work. > + */ > + if (byte == W1_SKIP_ROM) > + omap_hdq_break(hdq_data); > + > ret = mutex_lock_interruptible(&hdq_data->hdq_mutex); > if (ret < 0) { > dev_dbg(hdq_data->dev, "Could not acquire mutex\n"); > @@ -551,6 +673,7 @@ static int omap_hdq_probe(struct platform_device *pdev) > struct resource *res; > int ret, irq; > u8 rev; > + const char *mode; > > hdq_data = devm_kzalloc(dev, sizeof(*hdq_data), GFP_KERNEL); > if (!hdq_data) { > @@ -567,10 +690,21 @@ static int omap_hdq_probe(struct platform_device *pdev) > return PTR_ERR(hdq_data->hdq_base); > > hdq_data->hdq_usecount = 0; > + hdq_data->rrw = 0; > mutex_init(&hdq_data->hdq_mutex); > > pm_runtime_enable(&pdev->dev); > - pm_runtime_get_sync(&pdev->dev); > + ret = pm_runtime_get_sync(&pdev->dev); > + if (ret < 0) { > + dev_dbg(&pdev->dev, "pm_runtime_get_sync failed\n"); > + goto err_w1; > + } > + > + ret = _omap_hdq_reset(hdq_data); > + if (ret) { > + dev_dbg(&pdev->dev, "reset failed\n"); > + return -EINVAL; > + } > > rev = hdq_reg_in(hdq_data, OMAP_HDQ_REVISION); > dev_info(&pdev->dev, "OMAP HDQ Hardware Rev %c.%c. Driver in %s mode\n", > @@ -594,6 +728,15 @@ static int omap_hdq_probe(struct platform_device *pdev) > > pm_runtime_put_sync(&pdev->dev); > > + ret = of_property_read_string(pdev->dev.of_node, "ti,mode", &mode); > + if (ret < 0 || !strcmp(mode, "hdq")) { > + hdq_data->mode = 0; > + omap_w1_master.search = omap_w1_search_bus; > + } else { > + hdq_data->mode = 1; > + omap_w1_master.triplet = omap_w1_triplet; > + } > + > omap_w1_master.data = hdq_data; > > ret = w1_add_master_device(&omap_w1_master); > @@ -635,8 +778,8 @@ static int omap_hdq_remove(struct platform_device *pdev) > module_platform_driver(omap_hdq_driver); > > module_param(w1_id, int, S_IRUSR); > -MODULE_PARM_DESC(w1_id, "1-wire id for the slave detection"); > +MODULE_PARM_DESC(w1_id, "1-wire id for the slave detection in HDQ mode"); > > MODULE_AUTHOR("Texas Instruments"); > -MODULE_DESCRIPTION("HDQ driver Library"); > +MODULE_DESCRIPTION("HDQ-1W driver Library"); > MODULE_LICENSE("GPL"); > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/