From: Peter Rosin <peda@axentia.se>
To: Pankaj Bansal <pankaj.bansal@nxp.com>, Leo Li <leoyang.li@nxp.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux
Date: Thu, 21 Feb 2019 16:06:36 +0000 [thread overview]
Message-ID: <f48947f4-ec86-6877-9c88-414cb0f4dca9@axentia.se> (raw)
In-Reply-To: <VI1PR0401MB24962C5D61CB8B4BA08DA491F17E0@VI1PR0401MB2496.eurprd04.prod.outlook.com>
On 2019-02-21 15:17, Pankaj Bansal wrote:
>
>
>> -----Original Message-----
>> From: Peter Rosin [mailto:peda@axentia.se]
>> Sent: Thursday, 21 February, 2019 07:12 PM
>> To: Pankaj Bansal <pankaj.bansal@nxp.com>; Leo Li <leoyang.li@nxp.com>
>> Cc: linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH v2] drivers: mux: Add Generic regmap bitfield-based
>> multiplexer in mmio-mux
>>
>> Hi!
>>
>> Much better, thanks! One nit below:
>>
>> On 2019-02-21 13:48, Pankaj Bansal wrote:
>>> Generic register bitfield-based multiplexer that controls the
>>> multiplexer producer defined under a parent node.
>>> The driver corresponding to parent node provides register read/write
>>> capabilities.
>>>
>>> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
>>> ---
>>>
>>> Notes:
>>> V2:
>>> - removed seperate driver regmap.c and added the regmap function in
>> mmio.c
>>> based on compatible field, the syscon or regmap function would be called
>>> - Modified the KConfig as per Peter's comments
>>>
>>> drivers/mux/Kconfig | 12 ++++++------ drivers/mux/mmio.c | 6
>>> +++++-
>>> 2 files changed, 11 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig index
>>> 7659d6c5f718..e5c571fd232c 100644
>>> --- a/drivers/mux/Kconfig
>>> +++ b/drivers/mux/Kconfig
>>> @@ -46,14 +46,14 @@ config MUX_GPIO
>>> be called mux-gpio.
>>>
>>> config MUX_MMIO
>>> - tristate "MMIO register bitfield-controlled Multiplexer"
>>> - depends on (OF && MFD_SYSCON) || COMPILE_TEST
>>> + tristate "MMIO/Regmap register bitfield-controlled Multiplexer"
>>> + depends on OF || COMPILE_TEST
>>> help
>>> - MMIO register bitfield-controlled Multiplexer controller.
>>> + MMIO/Regmap register bitfield-controlled Multiplexer controller.
>>>
>>> - The driver builds multiplexer controllers for bitfields in a syscon
>>> - register. For N bit wide bitfields, there will be 2^N possible
>>> - multiplexer states.
>>> + The driver builds multiplexer controllers for bitfields in either
>>> + a syscon register or a driver regmap register. For N bit wide
>>> + bitfields, there will be 2^N possible multiplexer states.
>>>
>>> To compile the driver as a module, choose M here: the module will
>>> be called mux-mmio.
>>> diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c index
>>> 935ac44aa209..37fbcde7f1fc 100644
>>> --- a/drivers/mux/mmio.c
>>> +++ b/drivers/mux/mmio.c
>>> @@ -28,6 +28,7 @@ static const struct mux_control_ops mux_mmio_ops = {
>>>
>>> static const struct of_device_id mux_mmio_dt_ids[] = {
>>> { .compatible = "mmio-mux", },
>>> + { .compatible = "reg-mux", },
>>> { /* sentinel */ }
>>> };
>>> MODULE_DEVICE_TABLE(of, mux_mmio_dt_ids); @@ -43,7 +44,10 @@
>> static
>>> int mux_mmio_probe(struct platform_device *pdev)
>>> int ret;
>>> int i;
>>>
>>> - regmap = syscon_node_to_regmap(np->parent);
>>> + if (of_device_is_compatible(np, "mmio-mux"))
>>> + regmap = syscon_node_to_regmap(np->parent);
>>> + else
>>> + regmap = dev_get_regmap(dev->parent, NULL);
>>
>> dev_get_regmap() returns NULL on failure, so I think you need something like:
>>
>> regmap = dev_get_regmap(dev->parent, NULL) ?: ERR_PTR(-
>> EINVAL);
>>
>> Or perhaps -ENODEV, -ENOTSUPP or something, not sure?
>
> How about IS_ERR_OR_NULL just below this, where right now IS_ERR is being used ?
I discarded that since you then have to specialize the error handling
to not just do "ret = PTR_ERR(regmap);"
But if you'd like to do something other than my minimal fix, then feel free
to make a proposal...
Cheers,
Peter
>
>>
>> Cheers,
>> Peter
>>
>>> if (IS_ERR(regmap)) {
>>> ret = PTR_ERR(regmap);
>>> dev_err(dev, "failed to get regmap: %d\n", ret);
>>>
>
prev parent reply other threads:[~2019-02-21 16:06 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-21 12:48 [PATCH v2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux Pankaj Bansal
2019-02-21 13:42 ` Peter Rosin
2019-02-21 14:17 ` Pankaj Bansal
2019-02-21 16:06 ` Peter Rosin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f48947f4-ec86-6877-9c88-414cb0f4dca9@axentia.se \
--to=peda@axentia.se \
--cc=leoyang.li@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pankaj.bansal@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).