| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20140729102531.GD21182@ulmo.nvidia.com>
Date: Tue, 29 Jul 2014 12:25:33 +0200
From: Thierry Reding <thierry.reding@...il.com>
To: Doug Anderson <dianders@...omium.org>
Cc: caesar <caesar.wang@...k-chips.com>, linux-pwm@...r.kernel.org,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] pwm: add this patch to support the new pwm of
Rockchip SoCs
On Sat, Jul 26, 2014 at 09:59:35PM -0700, Doug Anderson wrote:
> caesar,
>
> On Fri, Jul 25, 2014 at 3:29 AM, caesar <caesar.wang@...k-chips.com> wrote:
> > Hi Thierry,
> >
> >
> >
> >
> > 在 2014年07月21日 21:27, Thierry Reding 写道:
> >>
> >> On Mon, Jul 21, 2014 at 08:58:42PM +0800, caesar wrote:
> >>
> >>> 于 2014年07月21日 16:50, Thierry Reding 写道:
> >>>>
> >>>> On Sat, Jul 19, 2014 at 08:55:29PM +0800, Caesar Wang wrote:
> >>
> >> [...]
> >>>>>
> >>>>> struct rockchip_pwm_chip *pc;
> >>>>> struct resource *r;
> >>>>> int ret;
> >>>>> @@ -119,7 +182,10 @@ static int rockchip_pwm_probe(struct
> >>>>> platform_device *pdev)
> >>>>> return -ENOMEM;
> >>>>> r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >>>>> - pc->base = devm_ioremap_resource(&pdev->dev, r);
> >>>>> + if (!strcmp(of_id->compatible, "rockchip,vop-pwm"))
> >>>>> + pc->base = devm_ioremap(&pdev->dev, r->start,
> >>>>> resource_size(r));
> >>>>> + else
> >>>>> + pc->base = devm_ioremap_resource(&pdev->dev, r);
> >>>>
> >>>> Sorry, this still isn't an option. You really shouldn't remap I/O
> >>>> regions that other drivers may be using. You hinted at a shared register
> >>>> space during the review of the initial version. Can you provide more
> >>>> detail about what exactly the memory map looks like of the rk3288? Is
> >>>> there some kind of technical reference manual that I could look at? Or
> >>>> do you have a device tree extract that shows what the memory map looks
> >>>> like?
> >>>>
> >>>> Thierry
> >>>
> >>> Maybe,you can look at the ARM: dts: rk3288:
> >>>
> >>> https://github.com/rkchrome/kernel/blob/master/arch/arm/boot/dts/rk3288.dtsi
> >>> There is some lcdc and vop-pwm map address for rk3288.
> >>>
> >>> ,and you can look at the vop-introduce.pdf and vop-register.pdf in Annex.
> >>>
> >>> Maybe,I should put the vop-pwm in lcdc driver,but I don't hope do so it.
> >>>
> >>> Could you give a suggestion to solve it? Thanks.
> >>
> >> It looks like you could turn the lcdc device into an MFD device so that
> >> it can instantiate two devices, one for the display controller, the
> >> other for the PWM. Or perhaps it would even work with only a single
> >> child device.
> >>
> >> The device tree would become something like this:
> >>
> >> lcdc@...30000 {
> >> compatible = "rockchip,rk3288-lcdc";
> >> ...
> >>
> >> pwm@...301a0 {
> >> compatible = "rockchip,vop-pwm";
> >> ...
> >> };
> >> };
> >>
> >> And your driver would do something like:
> >>
> >> static const struct resource pwm_resources[] = {
> >> {
> >> .start = 0x1a0,
> >> .end = 0x1af,
> >> .flags = IORESOURCE_MEM,
> >> },
> >> };
> >>
> >> static const struct mfd_cell subdevices[] = {
> >> {
> >> .name = "pwm",
> >> .id = 1,
> >> .of_compatible = "rockchip,vop-pwm",
> >> .num_resources = ARRAY_SIZE(pwm_resources),
> >> .resources = pwm_resources,
> >> },
> >> };
> >>
> >> static int lcdc_probe(struct platform_device *pdev)
> >> {
> >> struct resource *regs;
> >> ...
> >>
> >> regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >>
> >> ...
> >>
> >> err = mfd_add_devices(&pdev->dev, 0, subdevices,
> >> ARRAY_SIZE(subdevices),
> >> regs, NULL, NULL);
> >> ...
> >> }
> >>
> >> Thierry
> >
> > Sorry,I might a little trouble for the changes.
> >
> > The driver changes only for lcdc? If that is the case,I suddenly don't know
> > how to do it ?
> >
> > Maybe,I didn't say it clearly.
> >
> > lcdc0: lcdc@...30000 | vop0pwm: pwm@...301a0
> > reg = <0xff930000 0x10000> | reg = <0xff9301a0 0x10>;
> >
> > The lcdc has to add resource's address from 0xff930000 to 0xff93ffff.
> >
> > When the pwm driver is loading vop0pwm. the "devm_ioremap_resource()" will
> > be used in probe();
> >
> > I think it will be occur a fail. because the resource [mem
> > 0xff9301a0-0xff9301af] has be requested by lcdc.
> > =>rockchip-pwm ff9301a0.pwm: can't request region for resource [mem
> > 0xff9301a0-0xff9301af]
> >
> > If I do the changes in pwm driver,do you have a other suggestion for it?
> > thanks.:-)
>
> Sorry if this is stupid (and I haven't tried it), but does "ranges"
> help solve this problem? AKA:
>
> lcdc@...30000 {
> compatible = "rockchip,rk3288-lcdc";
> reg = <0xff930000 0x10000>;
> #address-cells = <2>;
> #size-cells = <1>;
> ranges = <0 0xff9301a0 0x10>;
> ...
>
> pwm@0,0 {
> compatible = "rockchip,vop-pwm";
> reg = <0 0 0x10>;
> ...
> };
> };
>
> Does that avoid the failure? The lcdc driver would need to call
> of_platform_populate() to make the PWM show up.
If you add "simple-bus" to the lcdc compatible string, like so:
lcdc@...30000 {
compatible = "rockchip,rk3288-lcdc", "simple-bus";
...
};
Then of_platform_populate() will be called automatically.
Thierry
Content of type "application/pgp-signature" skipped
Powered by blists - more mailing lists