All of lore.kernel.org
 help / color / mirror / Atom feed
From: mfuzzey@parkeon.com (Martin Fuzzey)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/5] W1: Add device tree support to MXC onewire master.
Date: Wed, 16 Jan 2013 19:11:21 +0100	[thread overview]
Message-ID: <50F6ED49.9080306@parkeon.com> (raw)
In-Reply-To: <20130116163603.GA20855@e106331-lin.cambridge.arm.com>

On 16/01/13 17:36, Mark Rutland wrote:
> Hi,
>
> This looks nice and simple, I just have a couple of comment.
Thank you
> On Wed, Jan 16, 2013 at 09:48:23AM +0000, Martin Fuzzey wrote:
>> Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
>> ---
>>   drivers/w1/masters/mxc_w1.c |    9 ++++++++-
>>   1 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/w1/masters/mxc_w1.c b/drivers/w1/masters/mxc_w1.c
>> index 708a25f..949e566 100644
>> --- a/drivers/w1/masters/mxc_w1.c
>> +++ b/drivers/w1/masters/mxc_w1.c
>> @@ -186,9 +186,16 @@ static int mxc_w1_remove(struct platform_device *pdev)
>>   	return 0;
>>   }
>>   
>> +static struct of_device_id mxc_w1_dt_ids[] = {
>> +	{ .compatible = "fsl,imx21-owire" },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, mxc_w1_dt_ids);
>> +
>>   static struct platform_driver mxc_w1_driver = {
>>   	.driver = {
>> -		   .name = "mxc_w1",
>> +		.name = "mxc_w1",
>> +		.of_match_table = mxc_w1_dt_ids,
>>   	},
>>   	.probe = mxc_w1_probe,
>>   	.remove = mxc_w1_remove,
>>
> I see there's already a binding for gpio-based w1 -- "w1-gpio". Given this is
> already using the "w1" naming convention in a binding, I think it'd be worth
> using the compatible string "fsl,imx21-w1", for consistency (both with the
> other binding and the driver name).
I don't have any strong opinions on this but the other imx53 bindings
use the names used by Freescale in the hardware documentation hence
the "owire" rather than "w1".

I'm happy to change it if that is the consensus though.

> It's also be worth documenting (in
> Documentation/devicetree/bindings/w1/fsl-imx21-w1, presumably).
Ok, I thought that as the driver doesn't define any properties itself that
this wasn't required. However l see some other drivers do document in this
case.

So I will add a Documentation file.
> When adding bindings, it's also good to Cc devicetree-discuss.
Ok, added to -Cc

Martin

WARNING: multiple messages have this Message-ID (diff)
From: Martin Fuzzey <mfuzzey@parkeon.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Evgeniy Polyakov <zbr@ioremap.net>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	devicetree-discuss@lists.ozlabs.org,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/5] W1: Add device tree support to MXC onewire master.
Date: Wed, 16 Jan 2013 19:11:21 +0100	[thread overview]
Message-ID: <50F6ED49.9080306@parkeon.com> (raw)
In-Reply-To: <20130116163603.GA20855@e106331-lin.cambridge.arm.com>

On 16/01/13 17:36, Mark Rutland wrote:
> Hi,
>
> This looks nice and simple, I just have a couple of comment.
Thank you
> On Wed, Jan 16, 2013 at 09:48:23AM +0000, Martin Fuzzey wrote:
>> Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
>> ---
>>   drivers/w1/masters/mxc_w1.c |    9 ++++++++-
>>   1 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/w1/masters/mxc_w1.c b/drivers/w1/masters/mxc_w1.c
>> index 708a25f..949e566 100644
>> --- a/drivers/w1/masters/mxc_w1.c
>> +++ b/drivers/w1/masters/mxc_w1.c
>> @@ -186,9 +186,16 @@ static int mxc_w1_remove(struct platform_device *pdev)
>>   	return 0;
>>   }
>>   
>> +static struct of_device_id mxc_w1_dt_ids[] = {
>> +	{ .compatible = "fsl,imx21-owire" },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, mxc_w1_dt_ids);
>> +
>>   static struct platform_driver mxc_w1_driver = {
>>   	.driver = {
>> -		   .name = "mxc_w1",
>> +		.name = "mxc_w1",
>> +		.of_match_table = mxc_w1_dt_ids,
>>   	},
>>   	.probe = mxc_w1_probe,
>>   	.remove = mxc_w1_remove,
>>
> I see there's already a binding for gpio-based w1 -- "w1-gpio". Given this is
> already using the "w1" naming convention in a binding, I think it'd be worth
> using the compatible string "fsl,imx21-w1", for consistency (both with the
> other binding and the driver name).
I don't have any strong opinions on this but the other imx53 bindings
use the names used by Freescale in the hardware documentation hence
the "owire" rather than "w1".

I'm happy to change it if that is the consensus though.

> It's also be worth documenting (in
> Documentation/devicetree/bindings/w1/fsl-imx21-w1, presumably).
Ok, I thought that as the driver doesn't define any properties itself that
this wasn't required. However l see some other drivers do document in this
case.

So I will add a Documentation file.
> When adding bindings, it's also good to Cc devicetree-discuss.
Ok, added to -Cc

Martin

  reply	other threads:[~2013-01-16 18:11 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-16  9:48 [PATCH 0/5] W1: Support onewire master on i.MX53 Martin Fuzzey
2013-01-16  9:48 ` [PATCH 1/5] W1: Add device tree support to MXC onewire master Martin Fuzzey
2013-01-16 16:36   ` Mark Rutland
2013-01-16 18:11     ` Martin Fuzzey [this message]
2013-01-16 18:11       ` Martin Fuzzey
2013-01-16  9:48 ` [PATCH 2/5] ARM: i.MX53: Add clocks for i.mx53 " Martin Fuzzey
2013-01-16  9:48 ` [PATCH 3/5] DTS: Add device tree entry for onewire master on i.MX53 Martin Fuzzey
2013-01-16  9:48 ` [PATCH 4/5] W1: Add pinctrl support to MXC onewire master Martin Fuzzey
2013-01-16  9:48 ` [PATCH 5/5] W1: Convert MXC onewire master to devm_ functions Martin Fuzzey
2013-01-16 10:44 ` [PATCH 0/5] W1: Support onewire master on i.MX53 Sascha Hauer
2013-01-16 14:17 ` Evgeniy Polyakov

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=50F6ED49.9080306@parkeon.com \
    --to=mfuzzey@parkeon.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.