From mboxrd@z Thu Jan 1 00:00:00 1970 From: Srinivas Kandagatla Subject: Re: [PATCH v5 04/11] nvmem: Add a simple NVMEM framework for consumers Date: Thu, 18 Jun 2015 13:56:23 +0100 Message-ID: <5582BFF7.9070309@linaro.org> References: <1432226535-8640-1-git-send-email-srinivas.kandagatla@linaro.org> <1432226593-8817-1-git-send-email-srinivas.kandagatla@linaro.org> <5580A345.30204@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f180.google.com ([209.85.212.180]:37131 "EHLO mail-wi0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752262AbbFRM41 (ORCPT ); Thu, 18 Jun 2015 08:56:27 -0400 Received: by wicgi11 with SMTP id gi11so12892445wic.0 for ; Thu, 18 Jun 2015 05:56:26 -0700 (PDT) In-Reply-To: <5580A345.30204@codeaurora.org> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Stephen Boyd , linux-arm-kernel@lists.infradead.org Cc: devicetree@vger.kernel.org, arnd@arndb.de, Greg Kroah-Hartman , s.hauer@pengutronix.de, linux-kernel@vger.kernel.org, pantelis.antoniou@konsulko.com, Rob Herring , Mark Brown , Kumar Gala , mporter@konsulko.com, Maxime Ripard , linux-api@vger.kernel.org, linux-arm-msm@vger.kernel.org On 16/06/15 23:29, Stephen Boyd wrote: > On 05/21/2015 09:43 AM, Srinivas Kandagatla wrote: >> @@ -379,6 +380,351 @@ int nvmem_unregister(struct nvmem_device *nvmem) > [...] >> + >> + return nvmem; >> +} >> + >> +static int __nvmem_device_put(struct nvmem_device *nvmem) > > Why does this return int? It's not used anywhere. > I can remove it. >> +{ >> + module_put(nvmem->owner); >> + mutex_lock(&nvmem_mutex); >> + nvmem->users--; >> + mutex_unlock(&nvmem_mutex); >> + >> + return 0; >> +} >> + >> +static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id) >> +{ >> + struct nvmem_cell *cell = NULL; >> + struct nvmem_device *nvmem; >> + >> + nvmem = __nvmem_device_get(NULL, &cell, cell_id); >> + if (IS_ERR(nvmem)) >> + return (struct nvmem_cell *)nvmem; > > ERR_CAST? Will fix it. > >> + >> + >> + return cell; >> + >> +} >> + >> +static struct nvmem_cell *of_nvmem_cell_get(struct device_node *np, >> + const char *name) >> +{ >> + struct device_node *cell_np, *nvmem_np; >> + struct nvmem_cell *cell; >> + struct nvmem_device *nvmem; >> + const __be32 *addr; >> + int rval, len, index; >> + >> + index = of_property_match_string(np, "nvmem-cell-names", name); >> + >> + cell_np = of_parse_phandle(np, "nvmem-cell", index); >> + if (!cell_np) >> + return ERR_PTR(-EINVAL); >> + >> + nvmem_np = of_get_next_parent(cell_np); >> + if (!nvmem_np) >> + return ERR_PTR(-EINVAL); >> + >> + nvmem = __nvmem_device_get(nvmem_np, NULL, NULL); >> + if (IS_ERR(nvmem)) >> + return (struct nvmem_cell *)nvmem; > > ERR_CAST? Will fix it. > >> + >> + addr = of_get_property(cell_np, "reg", &len); >> + if (!addr || (len < 2 * sizeof(int))) { >> + dev_err(&nvmem->dev, "of_i2c: invalid reg on %s\n", > > huh? of_i2c? > :-) Will fix it. > >> + >> + /* if it's not end on byte boundary */ >> + if ((nbits + bit_offset) % BITS_PER_BYTE) { >> + /* setup the last byte with msb bits from nvmem */ >> + rc = regmap_raw_read(nvmem->regmap, >> + cell->offset + cell->bytes - 1, &v, 1); >> + *p |= GENMASK(7, (nbits + bit_offset) % BITS_PER_BYTE) & v; >> + >> + } >> + >> + return buf; >> +} >> + >> +/** >> + * nvmem_cell_write(): Write to a given nvmem cell > > This isn't kernel doc notation. It should be like > > nvmem_cell_write - Write to a given nvmem cell > Will fix as suggested by Sascha as foobar() - short function description of foobar Looks correct according to Documentation/kernel-doc-nano-HOWTO.txt >> + * >> + * @cell: nvmem cell to be written. >> + * @buf: Buffer to be written. >> + * @len: length of buffer to be written to nvmem cell. >> + * >> + * The return value will be an length of bytes written or non zero on failure. >> + */ >> +int nvmem_cell_write(struct nvmem_cell *cell, void *buf, ssize_t len) > [..] >> + >> static int nvmem_init(void) >> { >> return class_register(&nvmem_class); >> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h >> new file mode 100644 >> index 0000000..c3fa8c7 >> --- /dev/null >> +++ b/include/linux/nvmem-consumer.h >> @@ -0,0 +1,49 @@ >> +/* >> + * nvmem framework consumer. >> + * >> + * Copyright (C) 2015 Srinivas Kandagatla >> + * Copyright (C) 2013 Maxime Ripard >> + * >> + * This file is licensed under the terms of the GNU General Public >> + * License version 2. This program is licensed "as is" without any >> + * warranty of any kind, whether express or implied. >> + */ >> + >> +#ifndef _LINUX_NVMEM_CONSUMER_H >> +#define _LINUX_NVMEM_CONSUMER_H >> + >> +/* consumer cookie */ >> +struct nvmem_cell; >> + >> +#if IS_ENABLED(CONFIG_NVMEM) >> + >> +/* Cell based interface */ >> +struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *name); > > We should probably forward declare struct device in this file too. Yep I will do it. >