* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Laurent Pinchart @ 2013-07-25 11:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201307251300.49282.arnd@arndb.de>
Hi Arnd,
On Thursday 25 July 2013 13:00:49 Arnd Bergmann wrote:
> On Thursday 25 July 2013, Laurent Pinchart wrote:
> > On Wednesday 24 July 2013 20:32:03 Arnd Bergmann wrote:
> > > On Tuesday 23 July 2013, Tomasz Figa wrote:
> > > > On Tuesday 23 of July 2013 17:14:20 Alan Stern wrote:
> > > > > On Tue, 23 Jul 2013, Tomasz Figa wrote:
> > > > > > Where would you want to have those phy_address arrays stored?
> > > > > > There are no board files when booting with DT. Not even saying
> > > > > > that you don't need to use any hacky schemes like this when you
> > > > > > have DT that nicely specifies relations between devices.
> > > > >
> > > > > If everybody agrees DT has a nice scheme for specifying relations
> > > > > between devices, why not use that same scheme in the PHY core?
> > > >
> > > > It is already used, for cases when consumer device has a DT node
> > > > attached. In non-DT case this kind lookup translates loosely to
> > > > something that is being done in regulator framework - you can't bind
> > > > devices by pointers, because you don't have those pointers, so you
> > > > need to use device names.
> > >
> > > Sorry for jumping in to the middle of the discussion, but why does a new
> > > framework even bother defining an interface for board files?
> > >
> > > Can't we just drop any interfaces for platform data passing in the phy
> > > framework and put the burden of adding those to anyone who actually
> > > needs them? All the platforms we are concerned with here (exynos and
> > > omap, plus new platforms) can be booted using DT anyway.
> >
> > What about non-DT architectures such as MIPS (still widely used in
> > consumer networking equipments from what I've heard) ?
>
> * Vendors of such equipment have started moving on to ARM (e.g. Broadcom
> bcm47xx)
> * Some of the modern MIPS platforms are now using DT
> * Legacy platforms probably won't migrate to either DT or the generic PHY
> framework
>
> I'm not saying that we can't support legacy board files with the common PHY
> framework, but I'd expect things to be much easier if we focus on those
> platforms that are actively being worked on for now, to bring an end to the
> pointless API discussion.
Fair enough :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [V3 0/7] Enhance mmp display driver
From: jett zhou @ 2013-07-25 11:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACDDiy8OwYHbmYGwc0FV2NLVfu1BiUL9GYCmA2QEVVB5isFJGQ@mail.gmail.com>
Hi Jean
Does apply patch1~patch3 and patch6~patch7 have issue on your side? :)
Thanks
2013/7/14 jett zhou <jett.zhou@gmail.com>:
> Hi Jean
> As I checked the for-next branch, found patch4 and patch5 are
> already applied.
> As checked on my side, only apply patch1~patch3 and
> patch6~patch7 is ok, so think you can apply them except for patch4/5.
> If there is still problem, please let me know, I will check it then.
> Thanks
>
> 2013/7/9 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
>> HI,
>>
>> please rebase on the for-next branch as it does not apply
>>
>> Best Regards,
>> J.
>> On 19:27 Thu 27 Jun , Jett.Zhou wrote:
>>> Changelog:
>>> 1 Add more comments for the mmp_display rbswap usage
>>> according to Daniel's comments;
>>> 2 Add fix patch for ttc_dkb on rbswap;
>>> 3 Add more comments on the pitch usage and definition;
>>> 4 Combine the modification on pitch of graphic and video layer
>>> in one patch.
>>>
>>> Guoqing Li (2):
>>> video: mmp: rb swap setting update for mmp display
>>> video: mmp: optimize some register setting code
>>>
>>> Jett.Zhou (1):
>>> ARM: mmp: remove the legacy rbswap setting for ttc_dkb platform
>>>
>>> Jing Xiang (4):
>>> video: mmp: fix graphics/video layer enable/mask issue
>>> video: mmp: fix memcpy wrong size for mmp_addr issue
>>> video: mmp: calculate pitch value when fb set win
>>> video: mmp: add pitch info in mmp_win structure
>>>
>>> arch/arm/mach-mmp/ttc_dkb.c | 4 +-
>>> drivers/video/mmp/fb/mmpfb.c | 34 +++++++++++------
>>> drivers/video/mmp/hw/mmp_ctrl.c | 79 ++++++++++++++++++++++-----------------
>>> drivers/video/mmp/hw/mmp_ctrl.h | 5 ++
>>> include/video/mmp_disp.h | 6 +++
>>> 5 files changed, 79 insertions(+), 49 deletions(-)
>>>
>
>
>
> --
>
> ----------------------------------
> Best Regards
> Jett Zhou
--
----------------------------------
Best Regards
Jett Zhou
^ permalink raw reply
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Arnd Bergmann @ 2013-07-25 11:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2174304.5JlzJ583hP@avalon>
On Thursday 25 July 2013, Laurent Pinchart wrote:
> On Wednesday 24 July 2013 20:32:03 Arnd Bergmann wrote:
> > On Tuesday 23 July 2013, Tomasz Figa wrote:
> > > On Tuesday 23 of July 2013 17:14:20 Alan Stern wrote:
> > > > On Tue, 23 Jul 2013, Tomasz Figa wrote:
> > > > > Where would you want to have those phy_address arrays stored? There
> > > > > are no board files when booting with DT. Not even saying that you
> > > > > don't need to use any hacky schemes like this when you have DT that
> > > > > nicely specifies relations between devices.
> > > >
> > > > If everybody agrees DT has a nice scheme for specifying relations
> > > > between devices, why not use that same scheme in the PHY core?
> > >
> > > It is already used, for cases when consumer device has a DT node attached.
> > > In non-DT case this kind lookup translates loosely to something that is
> > > being done in regulator framework - you can't bind devices by pointers,
> > > because you don't have those pointers, so you need to use device names.
> >
> > Sorry for jumping in to the middle of the discussion, but why does a new
> > framework even bother defining an interface for board files?
> >
> > Can't we just drop any interfaces for platform data passing in the phy
> > framework and put the burden of adding those to anyone who actually needs
> > them? All the platforms we are concerned with here (exynos and omap, plus
> > new platforms) can be booted using DT anyway.
>
> What about non-DT architectures such as MIPS (still widely used in consumer
> networking equipments from what I've heard) ?
* Vendors of such equipment have started moving on to ARM (e.g. Broadcom bcm47xx)
* Some of the modern MIPS platforms are now using DT
* Legacy platforms probably won't migrate to either DT or the generic PHY framework
I'm not saying that we can't support legacy board files with the common
PHY framework, but I'd expect things to be much easier if we focus on those
platforms that are actively being worked on for now, to bring an end to the
pointless API discussion.
Arnd
^ permalink raw reply
* Re: [PATCH V2] driver/vga16fb.c: remove the unused variable "dev" of function vga16fb_destroy()
From: Paul Bolle @ 2013-07-25 10:22 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Gu Zheng, Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen,
Linux Fbdev development list, linux-kernel
In-Reply-To: <CAMuHMdXWpdTD4sHat9sm5pvXVeGNNKMF6eEN=bUK4qU0H6i1Aw@mail.gmail.com>
On Thu, 2013-07-25 at 12:11 +0200, Geert Uytterhoeven wrote:
> Please send a v3, which has the version info under the above "---",
> instead of making it a part of the patch description to be committed.
Grepping through my mail tells me this is the fourth time a patch has
been submitted to fix this trivial issue. Perhaps (one of) the
maintainers can indicate which patch, if any, the maintainers have taken
or will be taking.
Paul Bolle
^ permalink raw reply
* Re: [PATCH] driver/vga16fb.c: remove the unused variable "dev" of function vga16fb_destroy()
From: Luis Henriques @ 2013-07-25 10:20 UTC (permalink / raw)
To: Gu Zheng; +Cc: plagnioj, tomi.valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <51F09D5E.70304@cn.fujitsu.com>
Hi,
Gu Zheng <guz.fnst@cn.fujitsu.com> writes:
> Commit e21d2170f36602ae2708 removed the unnecessary platform_set_drvdata(),
> but left the variable "dev" unused, delete it.
>
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
> drivers/video/vga16fb.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
> index 830ded4..2827333 100644
> --- a/drivers/video/vga16fb.c
> +++ b/drivers/video/vga16fb.c
> @@ -1265,7 +1265,6 @@ static void vga16fb_imageblit(struct fb_info *info, const struct fb_image *image
>
> static void vga16fb_destroy(struct fb_info *info)
> {
> - struct platform_device *dev = container_of(info->device, struct platform_device, dev);
> iounmap(info->screen_base);
> fb_dealloc_cmap(&info->cmap);
> /* XXX unshare VGA regions */
I'm not sure if this fix isn't already queued as I sent a similar
patch a few days ago:
https://lkml.org/lkml/2013/7/10/524
Cheers,
--
Luis
^ permalink raw reply
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Laurent Pinchart @ 2013-07-25 10:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201307242032.03597.arnd@arndb.de>
Hi Arnd,
On Wednesday 24 July 2013 20:32:03 Arnd Bergmann wrote:
> On Tuesday 23 July 2013, Tomasz Figa wrote:
> > On Tuesday 23 of July 2013 17:14:20 Alan Stern wrote:
> > > On Tue, 23 Jul 2013, Tomasz Figa wrote:
> > > > Where would you want to have those phy_address arrays stored? There
> > > > are no board files when booting with DT. Not even saying that you
> > > > don't need to use any hacky schemes like this when you have DT that
> > > > nicely specifies relations between devices.
> > >
> > > If everybody agrees DT has a nice scheme for specifying relations
> > > between devices, why not use that same scheme in the PHY core?
> >
> > It is already used, for cases when consumer device has a DT node attached.
> > In non-DT case this kind lookup translates loosely to something that is
> > being done in regulator framework - you can't bind devices by pointers,
> > because you don't have those pointers, so you need to use device names.
>
> Sorry for jumping in to the middle of the discussion, but why does a *new*
> framework even bother defining an interface for board files?
>
> Can't we just drop any interfaces for platform data passing in the phy
> framework and put the burden of adding those to anyone who actually needs
> them? All the platforms we are concerned with here (exynos and omap, plus
> new platforms) can be booted using DT anyway.
What about non-DT architectures such as MIPS (still widely used in consumer
networking equipments from what I've heard) ?
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH V2] driver/vga16fb.c: remove the unused variable "dev" of function vga16fb_destroy()
From: Geert Uytterhoeven @ 2013-07-25 10:11 UTC (permalink / raw)
To: Gu Zheng
Cc: Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen,
Linux Fbdev development list, linux-kernel
In-Reply-To: <51F0F88D.8060903@cn.fujitsu.com>
On Thu, Jul 25, 2013 at 12:06 PM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> Commit e21d2170f36602ae2708 ("video: remove unnecessary
> platform_set_drvdata()") removed the unnecessary platform_set_drvdata(),
> but left the variable "dev" unused, delete it.
>
> v2:
> Following Geert's suggestion to make change log easier reading.
>
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
Please send a v3, which has the version info under the above "---",
instead of making it a part of the patch description to be committed.
Thanks again!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] video: xilinxfb: Fix compilation warning
From: Geert Uytterhoeven @ 2013-07-25 10:09 UTC (permalink / raw)
To: Michal Simek
Cc: Jingoo Han, Michal Simek, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Linux Fbdev development list,
linux-kernel@vger.kernel.org, Stepan Moskovchenko
In-Reply-To: <51EF8F27.3030003@monstr.eu>
On Wed, Jul 24, 2013 at 10:24 AM, Michal Simek <monstr@monstr.eu> wrote:
>> Just now, I tested that the same problem happens on ARM config.
>> Also, I solved it by adding '&' operator.
>>
>> '&' operator is necessary as below:
>>
>> dev_dbg(dev, "regs: phys=%pa, virt=%p\n",
>> &drvdata->regs_phys, drvdata->regs);
>> ^^^
>>
>> dev_dbg(dev, "fb: phys=%pa, virt=%p, size=%x\n",
>> &drvdata->fb_phys, drvdata->fb_virt, fbsize);
>> ^^^
>
> ok.
>
> The truth is as I said there are just 5 files in the whole kernel
> which are using %pa and if %pa is right way to go we should
> probably fix all off them.
"%pa" is a quite recent addition (v3.9).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH V2] driver/vga16fb.c: remove the unused variable "dev" of function vga16fb_destroy()
From: Gu Zheng @ 2013-07-25 10:06 UTC (permalink / raw)
To: plagnioj, tomi.valkeinen; +Cc: linux-fbdev, linux-kernel, Geert Uytterhoeven
Commit e21d2170f36602ae2708 ("video: remove unnecessary
platform_set_drvdata()") removed the unnecessary platform_set_drvdata(),
but left the variable "dev" unused, delete it.
v2:
Following Geert's suggestion to make change log easier reading.
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
drivers/video/vga16fb.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
index 830ded4..2827333 100644
--- a/drivers/video/vga16fb.c
+++ b/drivers/video/vga16fb.c
@@ -1265,7 +1265,6 @@ static void vga16fb_imageblit(struct fb_info *info, const struct fb_image *image
static void vga16fb_destroy(struct fb_info *info)
{
- struct platform_device *dev = container_of(info->device, struct platform_device, dev);
iounmap(info->screen_base);
fb_dealloc_cmap(&info->cmap);
/* XXX unshare VGA regions */
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply related
* Re: [PATCH] driver/vga16fb.c: remove the unused variable "dev" of function vga16fb_destroy()
From: Gu Zheng @ 2013-07-25 10:02 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen,
Linux Fbdev development list, linux-kernel
In-Reply-To: <CAMuHMdVxXLrUXQno+vtHQrT_mmc9C6q42HZ6F1TujLBeEZt+Mw@mail.gmail.com>
On 07/25/2013 05:58 PM, Geert Uytterhoeven wrote:
> On Thu, Jul 25, 2013 at 5:37 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>> Commit e21d2170f36602ae2708 removed the unnecessary platform_set_drvdata(),
>> but left the variable "dev" unused, delete it.
>
> When referring to another commit, please also include the oneline summary of
> the commit, to make it easier for people to see what it's about.
Got it, thanks for your reminder.:)
>
> E.g. "Commit e21d2170f36602ae2708 ("video: remove unnecessary
> platform_set_drvdata()") removed the unnecessary platform_set_drvdata(),
> but left the variable "dev" unused, delete it."
This is easier reading. I'll update it.
Regards,
Gu
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
^ permalink raw reply
* Re: [PATCH] driver/vga16fb.c: remove the unused variable "dev" of function vga16fb_destroy()
From: Geert Uytterhoeven @ 2013-07-25 9:58 UTC (permalink / raw)
To: Gu Zheng
Cc: Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen,
Linux Fbdev development list, linux-kernel
In-Reply-To: <51F09D5E.70304@cn.fujitsu.com>
On Thu, Jul 25, 2013 at 5:37 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> Commit e21d2170f36602ae2708 removed the unnecessary platform_set_drvdata(),
> but left the variable "dev" unused, delete it.
When referring to another commit, please also include the oneline summary of
the commit, to make it easier for people to see what it's about.
E.g. "Commit e21d2170f36602ae2708 ("video: remove unnecessary
platform_set_drvdata()") removed the unnecessary platform_set_drvdata(),
but left the variable "dev" unused, delete it."
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Mark Brown @ 2013-07-25 9:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201307242032.03597.arnd@arndb.de>
[-- Attachment #1: Type: text/plain, Size: 850 bytes --]
On Wed, Jul 24, 2013 at 08:32:03PM +0200, Arnd Bergmann wrote:
> Sorry for jumping in to the middle of the discussion, but why does a *new*
> framework even bother defining an interface for board files?
> Can't we just drop any interfaces for platform data passing in the phy
> framework and put the burden of adding those to anyone who actually needs
> them? All the platforms we are concerned with here (exynos and omap,
> plus new platforms) can be booted using DT anyway.
There's a bunch of non-DT architectures that are in active use (blackfin
for example) and I'd really hope that this is useful for some of them.
The pushback here was about the fact that the subsystem was doing odd
things with selecting device names which is odd in itself, I don't know
if that had bled over into the DT bindings but it sounded like it
might've done so.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Sylwester Nawrocki @ 2013-07-25 9:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201307242032.03597.arnd@arndb.de>
On 07/24/2013 08:32 PM, Arnd Bergmann wrote:
> On Tuesday 23 July 2013, Tomasz Figa wrote:
>> On Tuesday 23 of July 2013 17:14:20 Alan Stern wrote:
>>> On Tue, 23 Jul 2013, Tomasz Figa wrote:
>>>> Where would you want to have those phy_address arrays stored? There
>>>> are no board files when booting with DT. Not even saying that you
>>>> don't need to use any hacky schemes like this when you have DT that
>>>> nicely specifies relations between devices.
>>>
>>> If everybody agrees DT has a nice scheme for specifying relations
>>> between devices, why not use that same scheme in the PHY core?
>>
>> It is already used, for cases when consumer device has a DT node attached.
>> In non-DT case this kind lookup translates loosely to something that is
>> being done in regulator framework - you can't bind devices by pointers,
>> because you don't have those pointers, so you need to use device names.
>>
>
> Sorry for jumping in to the middle of the discussion, but why does a *new*
> framework even bother defining an interface for board files?
>
> Can't we just drop any interfaces for platform data passing in the phy
> framework and put the burden of adding those to anyone who actually needs
> them? All the platforms we are concerned with here (exynos and omap,
> plus new platforms) can be booted using DT anyway.
Indeed, I was also a bit surprised we still need non-dt support, since
migration to this generic PHY framework in case of exynos was solely
part of migration of the whole platform to DT.
Two of the drivers that are being converted are also used on s5pv210,
but there is currently no boards in mainline that would use devices
covered by those drivers and s5pv210 will very likely get DT support
in v3.13 anyway.
But it seems omap still needs non-dt support in the PHY framework.
---
Thanks,
Sylwester
^ permalink raw reply
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Arnd Bergmann @ 2013-07-25 7:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <51F0B373.5050907@ti.com>
On Thursday 25 July 2013, Kishon Vijay Abraham I wrote:
> On Thursday 25 July 2013 12:02 AM, Arnd Bergmann wrote:
> > On Tuesday 23 July 2013, Tomasz Figa wrote:
> >> On Tuesday 23 of July 2013 17:14:20 Alan Stern wrote:
> >>> On Tue, 23 Jul 2013, Tomasz Figa wrote:
> >>>> Where would you want to have those phy_address arrays stored? There
> >>>> are no board files when booting with DT. Not even saying that you
> >>>> don't need to use any hacky schemes like this when you have DT that
> >>>> nicely specifies relations between devices.
> >>>
> >>> If everybody agrees DT has a nice scheme for specifying relations
> >>> between devices, why not use that same scheme in the PHY core?
> >>
> >> It is already used, for cases when consumer device has a DT node attached.
> >> In non-DT case this kind lookup translates loosely to something that is
> >> being done in regulator framework - you can't bind devices by pointers,
> >> because you don't have those pointers, so you need to use device names.
> >>
> >
> > Sorry for jumping in to the middle of the discussion, but why does a new
> > framework even bother defining an interface for board files?
> >
> > Can't we just drop any interfaces for platform data passing in the phy
> > framework and put the burden of adding those to anyone who actually needs
> > them? All the platforms we are concerned with here (exynos and omap,
> > plus new platforms) can be booted using DT anyway.
>
> The OMAP3 platforms still needs to be supported for non-dt :-s
Can't you leave the existing PHY handling for legacy OMAP3 USB PHY
until they are all converted? I don't expect that to take a long time
now that the OMAP4 board files have been removed. Are there still
drivers without DT bindings that hold up the removal of the OMAP3
board files?
Otherwise I'd suggest delaying the phy subsystem by another merge window,
until that is resolved.
Arnd
^ permalink raw reply
* [PATCH] driver/vga16fb.c: remove the unused variable "dev" of function vga16fb_destroy()
From: Gu Zheng @ 2013-07-25 3:37 UTC (permalink / raw)
To: plagnioj, tomi.valkeinen; +Cc: linux-fbdev, linux-kernel
Commit e21d2170f36602ae2708 removed the unnecessary platform_set_drvdata(),
but left the variable "dev" unused, delete it.
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
drivers/video/vga16fb.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
index 830ded4..2827333 100644
--- a/drivers/video/vga16fb.c
+++ b/drivers/video/vga16fb.c
@@ -1265,7 +1265,6 @@ static void vga16fb_imageblit(struct fb_info *info, const struct fb_image *image
static void vga16fb_destroy(struct fb_info *info)
{
- struct platform_device *dev = container_of(info->device, struct platform_device, dev);
iounmap(info->screen_base);
fb_dealloc_cmap(&info->cmap);
/* XXX unshare VGA regions */
--
1.7.7
^ permalink raw reply related
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Arnd Bergmann @ 2013-07-24 18:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5977067.8rykRgjgre@flatron>
On Tuesday 23 July 2013, Tomasz Figa wrote:
> On Tuesday 23 of July 2013 17:14:20 Alan Stern wrote:
> > On Tue, 23 Jul 2013, Tomasz Figa wrote:
> > > Where would you want to have those phy_address arrays stored? There
> > > are no board files when booting with DT. Not even saying that you
> > > don't need to use any hacky schemes like this when you have DT that
> > > nicely specifies relations between devices.
> >
> > If everybody agrees DT has a nice scheme for specifying relations
> > between devices, why not use that same scheme in the PHY core?
>
> It is already used, for cases when consumer device has a DT node attached.
> In non-DT case this kind lookup translates loosely to something that is
> being done in regulator framework - you can't bind devices by pointers,
> because you don't have those pointers, so you need to use device names.
>
Sorry for jumping in to the middle of the discussion, but why does a *new*
framework even bother defining an interface for board files?
Can't we just drop any interfaces for platform data passing in the phy
framework and put the burden of adding those to anyone who actually needs
them? All the platforms we are concerned with here (exynos and omap,
plus new platforms) can be booted using DT anyway.
Arnd
^ permalink raw reply
* Re: [PATCH] video: xilinxfb: Fix compilation warning
From: Michal Simek @ 2013-07-24 8:24 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Michal Simek',
'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', linux-fbdev, linux-kernel,
'Stepan Moskovchenko'
In-Reply-To: <003a01ce8845$06df46e0$149dd4a0$@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 3494 bytes --]
On 07/24/2013 10:08 AM, Jingoo Han wrote:
> On Wednesday, July 24, 2013 4:42 PM, Michal Simek wrote:
>> On 07/24/2013 09:18 AM, Michal Simek wrote:
>>> On 07/24/2013 08:00 AM, Jingoo Han wrote:
>>>> On Wednesday, July 24, 2013 2:32 PM, Michal Simek wrote:
>>>>>
>>>>> regs_phys is phys_addr_t (u32 or u64).
>>>>> Lets retype it to u64.
>>>>>
>>>>> Fixes compilation warning introduced by:
>>>>> video: xilinxfb: Use drvdata->regs_phys instead of physaddr
>>>>> (sha1: c88fafef0135e1e1c3e23c3e32ccbeeabc587f81)
>>>>
>>>> CC'ed Stepan Moskovchenko
>>>>
>>>>
>>>> phys_addr_t is defined as below:
>>>>
>>>> #ifdef CONFIG_PHYS_ADDR_T_64BIT
>>>> typedef u64 phys_addr_t;
>>>> #else
>>>> typedef u32 phys_addr_t;
>>>> #endif
>>>>
>>>> According to 'Documentation/printk-formats.txt',
>>>> Physical addresses:
>>>> %pa 0x01234567 or 0x0123456789abcdef
>>>> For printing a phys_addr_t type (and its derivatives, such as
>>>> resource_size_t) which can vary based on build options, regardless of
>>>> the width of the CPU data path. Passed by reference.
>>>>
>>>> Thus, '%pa' option looks proper, instead of casting (unsigned long long).
>>>>
>>>> dev_dbg(dev, "regs: phys=%pa, virt=%p\n", drvdata->regs_phys,
>>>> drvdata->regs);
>>>>
>>>
>>> Ah. Wasn't aware about that.
>>> Will retest.
>>
>> On ppc44x_defconfig
>>
>> $ powerpc-unknown-linux-gnu-gcc --version
>> powerpc-unknown-linux-gnu-gcc (crosstool-NG-svn_unknown@20110406.104745) 4.3.2
>>
>> This fix
>> diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
>> index 79175a6..a9a1167 100644
>> --- a/drivers/video/xilinxfb.c
>> +++ b/drivers/video/xilinxfb.c
>> @@ -341,12 +341,12 @@ static int xilinxfb_assign(struct platform_device *pdev,
>>
>> if (drvdata->flags & BUS_ACCESS_FLAG) {
>> /* Put a banner in the log (for DEBUG) */
>> - dev_dbg(dev, "regs: phys=%llx, virt=%p\n",
>> - (unsigned long long)drvdata->regs_phys, drvdata->regs);
>> + dev_dbg(dev, "regs: phys=%pa, virt=%p\n",
>> + drvdata->regs_phys, drvdata->regs);
>> }
>> /* Put a banner in the log (for DEBUG) */
>> - dev_dbg(dev, "fb: phys=%llx, virt=%p, size=%x\n",
>> - (unsigned long long)drvdata->fb_phys, drvdata->fb_virt, fbsize);
>> + dev_dbg(dev, "fb: phys=%pa, virt=%p, size=%x\n",
>> + drvdata->fb_phys, drvdata->fb_virt, fbsize);
>>
>
> Hi Michal Simek,
>
> Just now, I tested that the same problem happens on ARM config.
> Also, I solved it by adding '&' operator.
>
> '&' operator is necessary as below:
>
> dev_dbg(dev, "regs: phys=%pa, virt=%p\n",
> &drvdata->regs_phys, drvdata->regs);
> ^^^
>
> dev_dbg(dev, "fb: phys=%pa, virt=%p, size=%x\n",
> &drvdata->fb_phys, drvdata->fb_virt, fbsize);
> ^^^
ok.
The truth is as I said there are just 5 files in the whole kernel
which are using %pa and if %pa is right way to go we should
probably fix all off them.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: [PATCH] video: xilinxfb: Fix compilation warning
From: Jingoo Han @ 2013-07-24 8:08 UTC (permalink / raw)
To: Michal Simek
Cc: 'Michal Simek',
'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', linux-fbdev, linux-kernel,
'Stepan Moskovchenko', Jingoo Han
In-Reply-To: <51EF8530.90809@monstr.eu>
On Wednesday, July 24, 2013 4:42 PM, Michal Simek wrote:
> On 07/24/2013 09:18 AM, Michal Simek wrote:
> > On 07/24/2013 08:00 AM, Jingoo Han wrote:
> >> On Wednesday, July 24, 2013 2:32 PM, Michal Simek wrote:
> >>>
> >>> regs_phys is phys_addr_t (u32 or u64).
> >>> Lets retype it to u64.
> >>>
> >>> Fixes compilation warning introduced by:
> >>> video: xilinxfb: Use drvdata->regs_phys instead of physaddr
> >>> (sha1: c88fafef0135e1e1c3e23c3e32ccbeeabc587f81)
> >>
> >> CC'ed Stepan Moskovchenko
> >>
> >>
> >> phys_addr_t is defined as below:
> >>
> >> #ifdef CONFIG_PHYS_ADDR_T_64BIT
> >> typedef u64 phys_addr_t;
> >> #else
> >> typedef u32 phys_addr_t;
> >> #endif
> >>
> >> According to 'Documentation/printk-formats.txt',
> >> Physical addresses:
> >> %pa 0x01234567 or 0x0123456789abcdef
> >> For printing a phys_addr_t type (and its derivatives, such as
> >> resource_size_t) which can vary based on build options, regardless of
> >> the width of the CPU data path. Passed by reference.
> >>
> >> Thus, '%pa' option looks proper, instead of casting (unsigned long long).
> >>
> >> dev_dbg(dev, "regs: phys=%pa, virt=%p\n", drvdata->regs_phys,
> >> drvdata->regs);
> >>
> >
> > Ah. Wasn't aware about that.
> > Will retest.
>
> On ppc44x_defconfig
>
> $ powerpc-unknown-linux-gnu-gcc --version
> powerpc-unknown-linux-gnu-gcc (crosstool-NG-svn_unknown@20110406.104745) 4.3.2
>
> This fix
> diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
> index 79175a6..a9a1167 100644
> --- a/drivers/video/xilinxfb.c
> +++ b/drivers/video/xilinxfb.c
> @@ -341,12 +341,12 @@ static int xilinxfb_assign(struct platform_device *pdev,
>
> if (drvdata->flags & BUS_ACCESS_FLAG) {
> /* Put a banner in the log (for DEBUG) */
> - dev_dbg(dev, "regs: phys=%llx, virt=%p\n",
> - (unsigned long long)drvdata->regs_phys, drvdata->regs);
> + dev_dbg(dev, "regs: phys=%pa, virt=%p\n",
> + drvdata->regs_phys, drvdata->regs);
> }
> /* Put a banner in the log (for DEBUG) */
> - dev_dbg(dev, "fb: phys=%llx, virt=%p, size=%x\n",
> - (unsigned long long)drvdata->fb_phys, drvdata->fb_virt, fbsize);
> + dev_dbg(dev, "fb: phys=%pa, virt=%p, size=%x\n",
> + drvdata->fb_phys, drvdata->fb_virt, fbsize);
>
Hi Michal Simek,
Just now, I tested that the same problem happens on ARM config.
Also, I solved it by adding '&' operator.
'&' operator is necessary as below:
dev_dbg(dev, "regs: phys=%pa, virt=%p\n",
&drvdata->regs_phys, drvdata->regs);
^^^
dev_dbg(dev, "fb: phys=%pa, virt=%p, size=%x\n",
&drvdata->fb_phys, drvdata->fb_virt, fbsize);
^^^
Best regards,
Jingoo Han
> return 0; /* success */
>
> Generates two warnings even it should be ok according to link to specification you sent.
> CC [M] drivers/video/xilinxfb.o
> drivers/video/xilinxfb.c: In function 'xilinxfb_assign':
> drivers/video/xilinxfb.c:344: warning: format '%p' expects type 'void *', but argument 4 has type
> 'phys_addr_t'
> drivers/video/xilinxfb.c:348: warning: format '%p' expects type 'void *', but argument 4 has type
> 'dma_addr_t'
>
> On microblaze toolchain I see the same warnings. (mmu_defconfig)
>
> I have also grepped the kernel and I see that it is used in 4 c files which seems to me
> weird because phy_addr_t or dma_addr_t are used on a lot of places.
>
> Thanks,
> Michal
>
> --
> Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
> Maintainer of Linux kernel - Xilinx Zynq ARM architecture
> Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
>
^ permalink raw reply
* Re: [PATCH] video: xilinxfb: Fix compilation warning
From: Michal Simek @ 2013-07-24 7:41 UTC (permalink / raw)
To: monstr
Cc: Jingoo Han, 'Michal Simek',
'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', linux-fbdev, linux-kernel,
'Stepan Moskovchenko'
In-Reply-To: <51EF7FBD.8070103@monstr.eu>
[-- Attachment #1: Type: text/plain, Size: 3286 bytes --]
On 07/24/2013 09:18 AM, Michal Simek wrote:
> On 07/24/2013 08:00 AM, Jingoo Han wrote:
>> On Wednesday, July 24, 2013 2:32 PM, Michal Simek wrote:
>>>
>>> regs_phys is phys_addr_t (u32 or u64).
>>> Lets retype it to u64.
>>>
>>> Fixes compilation warning introduced by:
>>> video: xilinxfb: Use drvdata->regs_phys instead of physaddr
>>> (sha1: c88fafef0135e1e1c3e23c3e32ccbeeabc587f81)
>>
>> CC'ed Stepan Moskovchenko
>>
>>
>> phys_addr_t is defined as below:
>>
>> #ifdef CONFIG_PHYS_ADDR_T_64BIT
>> typedef u64 phys_addr_t;
>> #else
>> typedef u32 phys_addr_t;
>> #endif
>>
>> According to 'Documentation/printk-formats.txt',
>> Physical addresses:
>> %pa 0x01234567 or 0x0123456789abcdef
>> For printing a phys_addr_t type (and its derivatives, such as
>> resource_size_t) which can vary based on build options, regardless of
>> the width of the CPU data path. Passed by reference.
>>
>> Thus, '%pa' option looks proper, instead of casting (unsigned long long).
>>
>> dev_dbg(dev, "regs: phys=%pa, virt=%p\n", drvdata->regs_phys,
>> drvdata->regs);
>>
>
> Ah. Wasn't aware about that.
> Will retest.
On ppc44x_defconfig
$ powerpc-unknown-linux-gnu-gcc --version
powerpc-unknown-linux-gnu-gcc (crosstool-NG-svn_unknown@20110406.104745) 4.3.2
This fix
diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index 79175a6..a9a1167 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -341,12 +341,12 @@ static int xilinxfb_assign(struct platform_device *pdev,
if (drvdata->flags & BUS_ACCESS_FLAG) {
/* Put a banner in the log (for DEBUG) */
- dev_dbg(dev, "regs: phys=%llx, virt=%p\n",
- (unsigned long long)drvdata->regs_phys, drvdata->regs);
+ dev_dbg(dev, "regs: phys=%pa, virt=%p\n",
+ drvdata->regs_phys, drvdata->regs);
}
/* Put a banner in the log (for DEBUG) */
- dev_dbg(dev, "fb: phys=%llx, virt=%p, size=%x\n",
- (unsigned long long)drvdata->fb_phys, drvdata->fb_virt, fbsize);
+ dev_dbg(dev, "fb: phys=%pa, virt=%p, size=%x\n",
+ drvdata->fb_phys, drvdata->fb_virt, fbsize);
return 0; /* success */
Generates two warnings even it should be ok according to link to specification you sent.
CC [M] drivers/video/xilinxfb.o
drivers/video/xilinxfb.c: In function 'xilinxfb_assign':
drivers/video/xilinxfb.c:344: warning: format '%p' expects type 'void *', but argument 4 has type 'phys_addr_t'
drivers/video/xilinxfb.c:348: warning: format '%p' expects type 'void *', but argument 4 has type 'dma_addr_t'
On microblaze toolchain I see the same warnings. (mmu_defconfig)
I have also grepped the kernel and I see that it is used in 4 c files which seems to me
weird because phy_addr_t or dma_addr_t are used on a lot of places.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply related
* Re: [PATCH] video: xilinxfb: Fix compilation warning
From: Michal Simek @ 2013-07-24 7:18 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Michal Simek',
'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', linux-fbdev, linux-kernel,
'Stepan Moskovchenko'
In-Reply-To: <007401ce8833$22ce0600$686a1200$@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 1438 bytes --]
On 07/24/2013 08:00 AM, Jingoo Han wrote:
> On Wednesday, July 24, 2013 2:32 PM, Michal Simek wrote:
>>
>> regs_phys is phys_addr_t (u32 or u64).
>> Lets retype it to u64.
>>
>> Fixes compilation warning introduced by:
>> video: xilinxfb: Use drvdata->regs_phys instead of physaddr
>> (sha1: c88fafef0135e1e1c3e23c3e32ccbeeabc587f81)
>
> CC'ed Stepan Moskovchenko
>
>
> phys_addr_t is defined as below:
>
> #ifdef CONFIG_PHYS_ADDR_T_64BIT
> typedef u64 phys_addr_t;
> #else
> typedef u32 phys_addr_t;
> #endif
>
> According to 'Documentation/printk-formats.txt',
> Physical addresses:
> %pa 0x01234567 or 0x0123456789abcdef
> For printing a phys_addr_t type (and its derivatives, such as
> resource_size_t) which can vary based on build options, regardless of
> the width of the CPU data path. Passed by reference.
>
> Thus, '%pa' option looks proper, instead of casting (unsigned long long).
>
> dev_dbg(dev, "regs: phys=%pa, virt=%p\n", drvdata->regs_phys,
> drvdata->regs);
>
Ah. Wasn't aware about that.
Will retest.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: [PATCH] video: xilinxfb: Fix compilation warning
From: Michal Simek @ 2013-07-24 7:17 UTC (permalink / raw)
To: Vivek Subbarao
Cc: Michal Simek, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
linux-fbdev, linux-kernel
In-Reply-To: <CAAY4GiVG-c63oJO1Kqv0Jf2Wa2Ba+hgLtHQYFqyuRW-f6hJ14Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 678 bytes --]
On 07/24/2013 08:05 AM, Vivek Subbarao wrote:
> Why is there a necessity to type cast to unsigned long long ? Whats the
> warning ?
>
Geerts: Build regressions/improvements in v3.11-rc2
+ drivers/video/xilinxfb.c: warning: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'phys_addr_t' [-Wformat]: => 344:3
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: [PATCH] video: xilinxfb: Fix compilation warning
From: Jingoo Han @ 2013-07-24 6:00 UTC (permalink / raw)
To: 'Michal Simek',
'Jean-Christophe Plagniol-Villard'
Cc: 'Michal Simek', 'Tomi Valkeinen', linux-fbdev,
linux-kernel, 'Stepan Moskovchenko', Jingoo Han
In-Reply-To: <ed64cda456be840147ae1f04bec6a99733f41c24.1374643896.git.michal.simek@xilinx.com>
On Wednesday, July 24, 2013 2:32 PM, Michal Simek wrote:
>
> regs_phys is phys_addr_t (u32 or u64).
> Lets retype it to u64.
>
> Fixes compilation warning introduced by:
> video: xilinxfb: Use drvdata->regs_phys instead of physaddr
> (sha1: c88fafef0135e1e1c3e23c3e32ccbeeabc587f81)
CC'ed Stepan Moskovchenko
phys_addr_t is defined as below:
#ifdef CONFIG_PHYS_ADDR_T_64BIT
typedef u64 phys_addr_t;
#else
typedef u32 phys_addr_t;
#endif
According to 'Documentation/printk-formats.txt',
Physical addresses:
%pa 0x01234567 or 0x0123456789abcdef
For printing a phys_addr_t type (and its derivatives, such as
resource_size_t) which can vary based on build options, regardless of
the width of the CPU data path. Passed by reference.
Thus, '%pa' option looks proper, instead of casting (unsigned long long).
dev_dbg(dev, "regs: phys=%pa, virt=%p\n", drvdata->regs_phys,
drvdata->regs);
Best regards,
Jingoo Han
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> ppc44x_defconfig
> Fixes regressions in v3.11-rc2
>
> ---
> drivers/video/xilinxfb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
> index f3d4a69..79175a6 100644
> --- a/drivers/video/xilinxfb.c
> +++ b/drivers/video/xilinxfb.c
> @@ -341,8 +341,8 @@ static int xilinxfb_assign(struct platform_device *pdev,
>
> if (drvdata->flags & BUS_ACCESS_FLAG) {
> /* Put a banner in the log (for DEBUG) */
> - dev_dbg(dev, "regs: phys=%x, virt=%p\n", drvdata->regs_phys,
> - drvdata->regs);
> + dev_dbg(dev, "regs: phys=%llx, virt=%p\n",
> + (unsigned long long)drvdata->regs_phys, drvdata->regs);
> }
> /* Put a banner in the log (for DEBUG) */
> dev_dbg(dev, "fb: phys=%llx, virt=%p, size=%x\n",
> --
> 1.8.2.3
>
^ permalink raw reply
* [PATCH] video: xilinxfb: Fix compilation warning
From: Michal Simek @ 2013-07-24 5:31 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard
Cc: Michal Simek, Tomi Valkeinen, linux-fbdev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]
regs_phys is phys_addr_t (u32 or u64).
Lets retype it to u64.
Fixes compilation warning introduced by:
video: xilinxfb: Use drvdata->regs_phys instead of physaddr
(sha1: c88fafef0135e1e1c3e23c3e32ccbeeabc587f81)
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
ppc44x_defconfig
Fixes regressions in v3.11-rc2
---
drivers/video/xilinxfb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index f3d4a69..79175a6 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -341,8 +341,8 @@ static int xilinxfb_assign(struct platform_device *pdev,
if (drvdata->flags & BUS_ACCESS_FLAG) {
/* Put a banner in the log (for DEBUG) */
- dev_dbg(dev, "regs: phys=%x, virt=%p\n", drvdata->regs_phys,
- drvdata->regs);
+ dev_dbg(dev, "regs: phys=%llx, virt=%p\n",
+ (unsigned long long)drvdata->regs_phys, drvdata->regs);
}
/* Put a banner in the log (for DEBUG) */
dev_dbg(dev, "fb: phys=%llx, virt=%p, size=%x\n",
--
1.8.2.3
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply related
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Mark Brown @ 2013-07-23 23:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130723194423.GA22984@kroah.com>
[-- Attachment #1: Type: text/plain, Size: 4782 bytes --]
On Tue, Jul 23, 2013 at 12:44:23PM -0700, Greg KH wrote:
> On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote:
> > statement. In any case this is why the APIs doing lookups do the
> > lookups in the context of the requesting device - devices ask for
> > whatever name they use locally.
> What do you mean by "locally"?
Within themselves - for example a regulator consumer asks for a given
supply on the device in terms of the supply names the device has.
> The problem with the api was that the phy core wanted a id and a name to
> create a phy, and then later other code was doing a "lookup" based on
> the name and id (mushed together), because it "knew" that this device
> was the one it wanted.
Ah, that sounds like the API is missing a component to link things
together. But I could be wrong. What I would expect to see is that the
consumer says "I want the PHY called X" and the PHY driver says "I
provide this set of PHYs" with a layer in between that plugs those
together. This would normally involve talking about the parent device
rather than the PHY itself.
> I think, that if you create a device, then just carry around the pointer
> to that device (in this case a phy) and pass it to whatever other code
> needs it. No need to do lookups on "known names" or anything else, just
> normal pointers, with no problems for multiple devices, busses, or
> naming issues.
I think you're not really talking about the lookup API at all here but
rather about one way in which the matching code can be written. What
everything *really* wants to do is work in terms of resources namespaced
within struct devices since every bit of hardware in the system should
have one of those it can use and if you have a struct device you can do
useful things like call dev_printk() and find the device tree data to do
device tree based lookups.
Unfortunately for a number of buses even when statically registering the
struct device doesn't get allocated until the device is probed so what
everyone fell back on doing was using dev_name() in cases where the
struct device wasn't there yet, or just always using it for consistency
since for most of the affected buses dev_name() is fixed for human
interface reasons. I think this is the issue you're concerned about
here since if the dev_name() is dynamically allocated this breaks down.
This only affects board files, DT and ACPI can both use their own data
structures to do the mapping.
I had thought you were talking about picking the names that the
consumers use (which isn't actually that big a deal, it's just a bit
annoying for the clock API).
> > It's adding platform data in the first place that gets tedious - and of
> > course there's also DT and ACPI to worry about, it's not just a case of
> > platform data and then you're done. Pushing the lookup into library
> > code means that drivers don't have to worry about any of this stuff.
> I agree, so just pass around the pointer to the phy and all is good. No
> need to worry about DT or ACPI or anything else.
No, in practice passing around the pointer gets tricky if you're using
something other than board files (or even are doing any kind of dynamic
stuff with board files) since the two devices need to find each other
and if you're using platform data then the code doing the matching has
to know about the platform data for every device it might need to match
which is just miserable.
Something would need to do something like allocate the PHY objects and
then arrange for them to be passed to both provider and consumer devices
prior to those being registered, knowing where to place the pointers in
the platform data for each device. This is straightforward with board
files but not otherwise, people have tried this before.
> > For most of the APIs doing this there is a clear and unambiguous name in
> > the hardware that can be used (and for hardware process reasons is
> > unlikely to get changed). The major exception to this is the clock API
> > since it is relatively rare to have clear, segregated IP level
> > information for IPs baked into larger chips. The other APIs tend to be
> > establishing chip to chip links.
> The clock api is having problems with multiple "names" due to dynamic
> devices from what I was told. I want to prevent the PHY interface from
> having that same issue.
I think the underlying issue here is that we don't have a good enough
general way for board files (or other C code but mostly them) to talk
about devices prior to their being registered - rather than have the
pointer you're talking about be the PHY object itself have the pointer
be something which allows us to match the struct device when it's
created. This should be transparent to drivers and would be usable by
all the existing APIs.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Tomasz Figa @ 2013-07-23 21:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <Pine.LNX.4.44L0.1307231708020.1304-100000@iolanthe.rowland.org>
On Tuesday 23 of July 2013 17:14:20 Alan Stern wrote:
> On Tue, 23 Jul 2013, Tomasz Figa wrote:
> > > If you want to keep the phy struct completely separate from the
> > > board
> > > file, there's an easy way to do it. Let's say the board file knows
> > > about N different PHYs in the system. Then you define an array of N
> > > pointers to phys:
> > >
> > > struct phy *(phy_address[N]);
> > >
> > > In the platform data for both PHY j and its controller, store
> > >
> > > &phy_address[j]. The PHY provider passes this cookie to phy_create:
> > > cookie = pdev->dev.platform_data;
> > > ret = phy_create(phy, cookie);
> > >
> > > and phy_create simply stores: *cookie = phy. The PHY consumer does
> > >
> > > much the same the same thing:
> > > cookie = pdev->dev.platform_data;
> > > phy = phy_get(cookie);
> > >
> > > phy_get returns *cookie if it isn't NULL, or an ERR_PTR otherwise.
> >
> > OK, this can work. Again, just technically, because it's rather ugly.
>
> There's no reason the phy_address things have to be arrays. A separate
> individual pointer for each PHY would work just as well.
>
> > Where would you want to have those phy_address arrays stored? There
> > are no board files when booting with DT. Not even saying that you
> > don't need to use any hacky schemes like this when you have DT that
> > nicely specifies relations between devices.
>
> If everybody agrees DT has a nice scheme for specifying relations
> between devices, why not use that same scheme in the PHY core?
It is already used, for cases when consumer device has a DT node attached.
In non-DT case this kind lookup translates loosely to something that is
being done in regulator framework - you can't bind devices by pointers,
because you don't have those pointers, so you need to use device names.
> > Anyway, board file should not be considered as a method to exchange
> > data between drivers. It should be used only to pass data from it to
> > drivers, not the other way. Ideally all data in a board file should
> > be marked as const and __init and dropped after system
> > initialization.
>
> The phy_address things don't have to be defined or allocated in the
> board file; they could be set up along with the platform data.
There is no platform data when booting with DT.
> In any case, this was simply meant to be a suggestion to show that it
> is relatively easy to do what you need without using name or ID
> strings.
Sure. It's good to have different options discussed as well.
Best regards,
Tomasz
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox