mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Michal Simek <michal.simek@xilinx.com>
To: "Linus Walleij" <linus.walleij@linaro.org>,
	"Sören Brinkmann" <soren.brinkmann@xilinx.com>
Cc: "linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Michal Simek <michal.simek@xilinx.com>
Subject: Re: [RFC] pinctrl driver for Zynq
Date: Tue, 7 Oct 2014 13:35:23 +0200	[thread overview]
Message-ID: <90d8a43bab8a4bbfa018b497ce2d6c4d@BY2FFO11FD002.protection.gbl> (raw)
In-Reply-To: <CACRpkdaZ2Yy7=CcOjnsQ0K7KYTN=wHaeZTDNxpvnf+axLLLJ_g@mail.gmail.com>

Hi Linus,

Soren will reply it I believe just some explanation from me.

>> diff --git a/arch/arm/boot/dts/zynq-zc706.dts b/arch/arm/boot/dts/zynq-zc706.dts
>> index 4cc9913078cd..1ae9bcaee252 100644
>> --- a/arch/arm/boot/dts/zynq-zc706.dts
>> +++ b/arch/arm/boot/dts/zynq-zc706.dts
>> @@ -33,11 +33,20 @@
>>  &gem0 {
> 
> I'm not familiar with this syntax of putting an ampersand in front
> of a node like that. What does that mean? To me ampersands
> are phandles :-/

It just uses node label as reference and add some properties to it.
It means gem0 node is in dtsi and in this dts we want to extend it
by board specific information.

...

>> +       pctrl = devm_kzalloc(&pdev->dev, sizeof(*pctrl), GFP_KERNEL);
>> +       if (!pctrl)
>> +               return -ENOMEM;
>> +
>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +
>> +       slcr = of_get_parent(pdev->dev.of_node);
>> +       if (slcr->data) {
>> +               pctrl->regs = (__force void __iomem *)slcr->data + res->start;
> 
> This looks weird. Use DT parsing functions and accessors, no funny
> business like this. The res-start to the device should be the real
> physical address, not a relative base with offset, dunno what happened
> here but it is wrong.

Soren already has follow up version with is using regmap interface because slcr
is parent and it is syscon. That's why this code will go away.

Thanks,
Michal


  reply	other threads:[~2014-10-07 11:35 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-24 21:09 Sören Brinkmann
2014-09-25  8:17 ` Steffen Trumtrar
2014-09-25 16:02   ` Sören Brinkmann
2014-10-07 11:07     ` Linus Walleij
2014-10-07 11:05 ` Linus Walleij
2014-10-07 11:35   ` Michal Simek [this message]
2014-10-07 16:37   ` Sören Brinkmann
2014-10-08 21:30   ` Sören Brinkmann

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=90d8a43bab8a4bbfa018b497ce2d6c4d@BY2FFO11FD002.protection.gbl \
    --to=michal.simek@xilinx.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=soren.brinkmann@xilinx.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

all inboxes | Powered by JetHome®