From patchwork Mon Oct 26 13:37:53 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Srinivas Kandagatla X-Patchwork-Id: 55536 Delivered-To: patch@linaro.org Received: by 10.112.59.35 with SMTP id w3csp1209249lbq; Mon, 26 Oct 2015 06:37:59 -0700 (PDT) X-Received: by 10.66.124.200 with SMTP id mk8mr21438437pab.25.1445866679147; Mon, 26 Oct 2015 06:37:59 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id bv3si53256498pbd.105.2015.10.26.06.37.58; Mon, 26 Oct 2015 06:37:59 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of devicetree-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of devicetree-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=devicetree-owner@vger.kernel.org; dkim=neutral (body hash did not verify) header.i=@linaro_org.20150623.gappssmtp.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753948AbbJZNh6 (ORCPT + 6 others); Mon, 26 Oct 2015 09:37:58 -0400 Received: from mail-wi0-f181.google.com ([209.85.212.181]:34037 "EHLO mail-wi0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750859AbbJZNh5 (ORCPT ); Mon, 26 Oct 2015 09:37:57 -0400 Received: by wikq8 with SMTP id q8so165293224wik.1 for ; Mon, 26 Oct 2015 06:37:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro_org.20150623.gappssmtp.com; s=20150623; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=OU4IupbukRZiwdfZIWgL4TwcC/oWHgldSLH+RYj9tr8=; b=OuxEiDW4aOe0Duf2i1TX9Nmf+FXjYWp1aSIERbZneeoZwEK7cCY3LQpY1A3b06a9EC efO3N+nJMRBLdYs/UOvFRem50bY2Gc/MPXHYNlVEj5jqU5U+l3L1mZv2K2CegVziDmO1 xmb6bSx9JF5P7l7DRzpdSzo5+g+zLUtK7c7a4EQqtZK1ba8mKJqqiJTeMWu3vZji67Oj xKoT6rFDNpuin5hDuJ6p36+z5KHIuE1LMCa8jZdhhsnyu4Aw/vAo5eBVSyPESsQ4+Chu hZ5jAbTVsYG4XD4FgQUesUWZuzWM78MqFSYLGKYuCUGDpt+6SpxzQk/60aF3Qcnjg7lU 9Njw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=OU4IupbukRZiwdfZIWgL4TwcC/oWHgldSLH+RYj9tr8=; b=PUDn2npfJoyuMsFDN6aiBd4v0ilCHH6wBznO/iGTozB8Y43SVwpKhFFVdHxKIeeKZb 2FDtOZDR29oEyoWHKFcQocVJfTpfEbXQ/+/VEN3fm9cWODyzoknJFOhIRjOED6P8UfBu w6bGTVQy0sY0Ru/mQJ9F3Z3hHHtHXpMozy+Ghi15WIIL6VpMvS2qzxxfB4VLZIdRVkFV sSI2SBPzpnKYkcfKAl39gn28iP+0ZNQcuC/O12UboVwpxvQYpvth28BoGkquwxFKUvPI LGSwFpXXTgWbvJK8aJpcPTjXC/FQEmaTPmkfiqdwFSeSengrMnftlrgxV64m/OkDmVzD MrSQ== X-Gm-Message-State: ALoCoQmLqjDcy7ji8/i8N2or7GsqXLT1bGOCESPcLw4lyPThBbedkUERANRQFTwMlbv0mcHuRO8F X-Received: by 10.180.205.145 with SMTP id lg17mr21188725wic.67.1445866675332; Mon, 26 Oct 2015 06:37:55 -0700 (PDT) Received: from [192.168.1.22] (host-92-13-246-184.as43234.net. [92.13.246.184]) by smtp.googlemail.com with ESMTPSA id q11sm16807385wik.1.2015.10.26.06.37.53 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 26 Oct 2015 06:37:54 -0700 (PDT) Message-ID: <562E2CB1.80706@linaro.org> Date: Mon, 26 Oct 2015 13:37:53 +0000 From: Srinivas Kandagatla User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Joachim Eastwood , Ariel D'Alessandro CC: "linux-arm-kernel@lists.infradead.org" , devicetree@vger.kernel.org, Maxime Ripard , Ezequiel Garcia , Kumar Gala , Ian Campbell , Mark Rutland , Pawel Moll , Rob Herring Subject: Re: [PATCH v2 2/4] nvmem: NXP LPC18xx EEPROM memory NVMEM driver References: <1445275946-32653-1-git-send-email-ariel@vanguardiasur.com.ar> <1445275946-32653-3-git-send-email-ariel@vanguardiasur.com.ar> In-Reply-To: Sender: devicetree-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On 24/10/15 23:04, Joachim Eastwood wrote: > Hi Ariel, > > On 19 October 2015 at 19:32, Ariel D'Alessandro > wrote: >> This commit adds support for NXP LPC18xx EEPROM memory found in NXP >> LPC185x/3x and LPC435x/3x/2x/1x devices. >> >> EEPROM size is 16384 bytes and it can be entirely read and >> written/erased with 1 word (4 bytes) granularity. The last page >> (128 bytes) contains the EEPROM initialization data and is not writable. >> >> Erase/program time is less than 3ms. The EEPROM device requires a >> ~1500 kHz clock (min 800 kHz, max 1600 kHz) that is generated dividing >> the system bus clock by the division factor, contained in the divider >> register (minus 1 encoded). >> >> Signed-off-by: Ariel D'Alessandro >> --- >> drivers/nvmem/Kconfig | 9 ++ >> drivers/nvmem/Makefile | 2 + >> drivers/nvmem/lpc18xx_eeprom.c | 266 +++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 277 insertions(+) >> create mode 100644 drivers/nvmem/lpc18xx_eeprom.c > >> +static int lpc18xx_eeprom_gather_write(void *context, const void *reg, >> + size_t reg_size, const void *val, >> + size_t val_size) >> +{ >> + struct lpc18xx_eeprom_dev *eeprom = context; >> + unsigned int offset = *(u32 *)reg; >> + >> + /* 3 ms of erase/program time between each writing */ >> + while (val_size) { >> + writel(*(u32 *)val, eeprom->mem_base + offset); >> + usleep_range(3000, 4000); >> + val_size -= eeprom->val_bytes; >> + val += eeprom->val_bytes; >> + offset += eeprom->val_bytes; >> + } > > What happens here if 'val_size' is less than 4 or not dividable by 4? > Same thing for 'offset'. > > I tested the driver from sysfs by writing strings into the nvmem-file > with echo. Writing a string not dividable by 4 seems to hang the > system. > I think I know the issue here: Could you try this patch: -------------------------------->cut<---------------------------------- From 8cae10eff8ea8da9c5a8058ff75abeeddd8a8224 Mon Sep 17 00:00:00 2001 From: Srinivas Kandagatla Date: Mon, 26 Oct 2015 13:30:24 +0000 Subject: [PATCH] nvmem: core: return error for non word aligned bytes nvmem providers have restrictions on register strides, so return error code when users attempt to read/write buffers with sizes which are not aligned to the word boundary. Without this patch the userspace would continue to try as it does not get any error from the nvmem core, resulting in a hang. Signed-off-by: Srinivas Kandagatla --- drivers/nvmem/core.c | 6 ++++++ 1 file changed, 6 insertions(+) @@ -95,6 +98,9 @@ static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj, if (pos >= nvmem->size) return 0; + if (count < nvmem->word_size) + return -EINVAL; + if (pos + count > nvmem->size) count = nvmem->size - pos; -- 1.9.1 -------------------------------->cut<---------------------------------- --srini > >> +static int lpc18xx_eeprom_read(void *context, const void *reg, size_t reg_size, >> + void *val, size_t val_size) >> +{ >> + struct lpc18xx_eeprom_dev *eeprom = context; >> + unsigned int offset = *(u32 *)reg; >> + >> + while (val_size) { >> + *(u32 *)val = readl(eeprom->mem_base + offset); >> + val_size -= eeprom->val_bytes; >> + val += eeprom->val_bytes; >> + offset += eeprom->val_bytes; >> + } >> + >> + return 0; >> +} > > Same comments as for lpc18xx_eeprom_gather_write(). > > >> +static const struct of_device_id lpc18xx_eeprom_of_match[] = { >> + { .compatible = "nxp,lpc1857-eeprom" }, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(of, lpc18xx_eeprom_of_match); > > nit: It's usual to place of_device_id struct just above the > platform_driver struct. > > >> + eeprom->val_bytes = lpc18xx_regmap_config.val_bits / 8; >> + eeprom->reg_bytes = lpc18xx_regmap_config.reg_bits / 8; > > There is a BITS_PER_BYTE define in bitops.h that you might want to use here. > > >> + /* >> + * Clock rate is generated by dividing the system bus clock by the >> + * division factor, contained in the divider register (minus 1 encoded). >> + */ >> + clk_rate = clk_get_rate(eeprom->clk); >> + clk_rate = DIV_ROUND_UP(clk_rate, LPC18XX_EEPROM_CLOCK_HZ) - 1; >> + lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_CLKDIV, clk_rate); >> + >> + /* >> + * Writing a single word to the page will start the erase/program cycle >> + * automatically >> + */ >> + lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_AUTOPROG, >> + LPC18XX_EEPROM_AUTOPROG_WORD); >> + >> + lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_PWRDWN, >> + LPC18XX_EEPROM_PWRDWN_NO); >> + >> + lpc18xx_regmap_config.max_register = resource_size(res) - 1; >> + lpc18xx_regmap_config.writeable_reg = lpc18xx_eeprom_writeable_reg; >> + lpc18xx_regmap_config.readable_reg = lpc18xx_eeprom_readable_reg; >> + >> + regmap = devm_regmap_init(dev, &lpc18xx_eeprom_bus, eeprom, >> + &lpc18xx_regmap_config); >> + if (IS_ERR(regmap)) { >> + dev_err(dev, "regmap init failed: %ld\n", PTR_ERR(regmap)); >> + ret = PTR_ERR(regmap); >> + goto err_clk; >> + } >> + >> + lpc18xx_nvmem_config.dev = dev; >> + >> + eeprom->nvmem = nvmem_register(&lpc18xx_nvmem_config); >> + if (IS_ERR(eeprom->nvmem)) { >> + ret = PTR_ERR(eeprom->nvmem); >> + goto err_clk; >> + } >> + >> + platform_set_drvdata(pdev, eeprom); >> + >> + return 0; >> + >> +err_clk: >> + clk_disable_unprepare(eeprom->clk); >> + >> + return ret; >> +} >> + >> +static int lpc18xx_eeprom_remove(struct platform_device *pdev) >> +{ >> + struct lpc18xx_eeprom_dev *eeprom = platform_get_drvdata(pdev); >> + >> + lpc18xx_eeprom_writel(eeprom, LPC18XX_EEPROM_PWRDWN, >> + LPC18XX_EEPROM_PWRDWN_YES); >> + >> + clk_disable_unprepare(eeprom->clk); >> + >> + return nvmem_unregister(eeprom->nvmem); > > Normally you do tear down in the reverse order of initialization. > > Consider what happens here when you power down and disable the clock > while there still are nvmem users of the eeprom. > > > regards, > Joachim Eastwood > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c index 6fd4e5a..9d11d98 100644 --- a/drivers/nvmem/core.c +++ b/drivers/nvmem/core.c @@ -70,6 +70,9 @@ static ssize_t bin_attr_nvmem_read(struct file *filp, struct kobject *kobj, if (pos >= nvmem->size) return 0; + if (count < nvmem->word_size) + return -EINVAL; + if (pos + count > nvmem->size) count = nvmem->size - pos;