Patchwork [GIT,PULL,v3] Renesas ARM-based SoC boards for v3.8 #2

login
register
mail settings
Submitter Simon Horman
Date Nov. 9, 2012, 7:31 a.m.
Message ID <1352446306-19945-1-git-send-email-horms@verge.net.au>
Download mbox
Permalink /patch/197961/
State New
Headers show

Pull-request

git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git boards

Comments

Simon Horman - Nov. 9, 2012, 7:31 a.m.
Hi Olof, Hi Arnd,

please consider the following board enhancements for 3.8.

* This pull request is based on a merge of
  a) The renesas/boards branch of the arm-soc tree
  b) The soc branch of the renesas tree,
     which I have sent a separate pull requst for

* "sh: clkfwk: add sh_clk_fsidiv_register()" is a driver patch
  which is a dependency of
  - ARM: shmobile: sh7372: use sh_clk_fsidiv_register() for FSI-DIV clocks
  - ARM: shmobile: r8a7740: add FSI-DVI clocks
  I have spoken to the driver maintainer, Paul Mundt, and he has indicated
  that he thinks it is best to merge all in one go and acked the patch for
  inclusion in this pull-request.

* The following patches
  - ARM: shmobile: use FSI driver's audio clock on ap4evb
  - ARM: shmobile: use FSI driver's audio clock on mackerel
  - ARM: shmobile: use FSI driver's audio clock on armadillo800eva
  Have a compile-time dependency on the following patch which is present in
  the for-next branch of Mark Brown's sound tree on kernel.org
  - ASoC: fsi: add master clock control functions
    (ab6f6d85210c4d0265cf48e9958c04e08595055a)

----------------------------------------------------------------
The following changes since commit ab3ff12a78a64b851ab22726ab99dca6ad37bf94:

  Merge branch 'soc' into boards (2012-11-08 17:49:35 +0900)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git boards

for you to fetch changes up to 12818d82ccad9868a820dbb193b4f9b797cd02d4:

  ARM: shmobile: use FSI driver's audio clock on ap4evb (2012-11-08 17:52:05 +0900)

----------------------------------------------------------------
Kuninori Morimoto (7):
      ARM: shmobile: marzen: add USB phy support
      ARM: shmobile: marzen: add USB EHCI driver support
      ARM: shmobile: marzen: add USB OHCI driver support
      ARM: shmobile: mackerel: enable DMAEngine on USB Host
      ARM: shmobile: use FSI driver's audio clock on armadillo800eva
      ARM: shmobile: use FSI driver's audio clock on mackerel
      ARM: shmobile: use FSI driver's audio clock on ap4evb

 arch/arm/configs/marzen_defconfig              |   14 +-
 arch/arm/mach-shmobile/Kconfig                 |    2 +
 arch/arm/mach-shmobile/board-ap4evb.c          |  139 +-----------------
 arch/arm/mach-shmobile/board-armadillo800eva.c |   38 +----
 arch/arm/mach-shmobile/board-mackerel.c        |   74 +---------
 arch/arm/mach-shmobile/board-marzen.c          |  186 +++++++++++++++++++++++-
 6 files changed, 205 insertions(+), 248 deletions(-)
Arnd Bergmann - Nov. 12, 2012, 9:11 p.m.
On Friday 09 November 2012, Simon Horman wrote:
> Hi Olof, Hi Arnd,
> 
> please consider the following board enhancements for 3.8.
> 
> * This pull request is based on a merge of
>   a) The renesas/boards branch of the arm-soc tree
>   b) The soc branch of the renesas tree,
>      which I have sent a separate pull requst for
> 
> * "sh: clkfwk: add sh_clk_fsidiv_register()" is a driver patch
>   which is a dependency of
>   - ARM: shmobile: sh7372: use sh_clk_fsidiv_register() for FSI-DIV clocks
>   - ARM: shmobile: r8a7740: add FSI-DVI clocks
>   I have spoken to the driver maintainer, Paul Mundt, and he has indicated
>   that he thinks it is best to merge all in one go and acked the patch for
>   inclusion in this pull-request.
> 
> * The following patches
>   - ARM: shmobile: use FSI driver's audio clock on ap4evb
>   - ARM: shmobile: use FSI driver's audio clock on mackerel
>   - ARM: shmobile: use FSI driver's audio clock on armadillo800eva
>   Have a compile-time dependency on the following patch which is present in
>   the for-next branch of Mark Brown's sound tree on kernel.org
>   - ASoC: fsi: add master clock control functions
>     (ab6f6d85210c4d0265cf48e9958c04e08595055a)

Hi Simon,

I've pulled this into a new next/boards2 branch because of the new dependency,
but I'm not entirely happy with the way that the dependency came in through
your tree.

It's generally ok to have external dependencies where they can't be avoided,
but please remember these rules:

* When you send a branch that has an external dependency, actually base your
  branch on top of the other one, so that it can be independently verified.
  If you have a dependency and send your patches without that one included,
  it's clear that your code can't be tested in the version you are sending,
  and it breaks any attempt to test just the arm-soc tree or your branch
  rather than the entire for-next tree.

* Make sure that the branch you depend on will not get rebased before it
  gets submitted to the mainline kernel.

* Always let the person that owns the dependency know that the changes in
  their tree are also included elsewhere and that things go bad if those
  changes get rebased after all, or won't make it into the merge window
  for some reason.

I have taken Mark on Cc to let him know about the dependency now, and I've
merged ab6f6d85210c4d0265cf48e9958c04e08595055a (which has only shmobile
specific ASoC patches) into the next/boards2 branch before merging your
branch. This is still not perfect because it breaks bisection, but it's
the best I could do aside from forcing you do do another round-trip.

	Arnd
Simon Horman - Nov. 13, 2012, 2:58 a.m.
On Mon, Nov 12, 2012 at 09:11:03PM +0000, Arnd Bergmann wrote:
> On Friday 09 November 2012, Simon Horman wrote:
> > Hi Olof, Hi Arnd,
> > 
> > please consider the following board enhancements for 3.8.
> > 
> > * This pull request is based on a merge of
> >   a) The renesas/boards branch of the arm-soc tree
> >   b) The soc branch of the renesas tree,
> >      which I have sent a separate pull requst for
> > 
> > * "sh: clkfwk: add sh_clk_fsidiv_register()" is a driver patch
> >   which is a dependency of
> >   - ARM: shmobile: sh7372: use sh_clk_fsidiv_register() for FSI-DIV clocks
> >   - ARM: shmobile: r8a7740: add FSI-DVI clocks
> >   I have spoken to the driver maintainer, Paul Mundt, and he has indicated
> >   that he thinks it is best to merge all in one go and acked the patch for
> >   inclusion in this pull-request.
> > 
> > * The following patches
> >   - ARM: shmobile: use FSI driver's audio clock on ap4evb
> >   - ARM: shmobile: use FSI driver's audio clock on mackerel
> >   - ARM: shmobile: use FSI driver's audio clock on armadillo800eva
> >   Have a compile-time dependency on the following patch which is present in
> >   the for-next branch of Mark Brown's sound tree on kernel.org
> >   - ASoC: fsi: add master clock control functions
> >     (ab6f6d85210c4d0265cf48e9958c04e08595055a)
> 
> Hi Simon,
> 
> I've pulled this into a new next/boards2 branch because of the new dependency,
> but I'm not entirely happy with the way that the dependency came in through
> your tree.
> 
> It's generally ok to have external dependencies where they can't be avoided,
> but please remember these rules:
> 
> * When you send a branch that has an external dependency, actually base your
>   branch on top of the other one, so that it can be independently verified.
>   If you have a dependency and send your patches without that one included,
>   it's clear that your code can't be tested in the version you are sending,
>   and it breaks any attempt to test just the arm-soc tree or your branch
>   rather than the entire for-next tree.
> 
> * Make sure that the branch you depend on will not get rebased before it
>   gets submitted to the mainline kernel.
> 
> * Always let the person that owns the dependency know that the changes in
>   their tree are also included elsewhere and that things go bad if those
>   changes get rebased after all, or won't make it into the merge window
>   for some reason.
> 
> I have taken Mark on Cc to let him know about the dependency now, and I've
> merged ab6f6d85210c4d0265cf48e9958c04e08595055a (which has only shmobile
> specific ASoC patches) into the next/boards2 branch before merging your
> branch. This is still not perfect because it breaks bisection, but it's
> the best I could do aside from forcing you do do another round-trip.

Thanks and sorry about that. I'll be more careful next time.
Mark Brown - Nov. 13, 2012, 5:36 a.m.
On Mon, Nov 12, 2012 at 09:11:03PM +0000, Arnd Bergmann wrote:

> * Always let the person that owns the dependency know that the changes in
>   their tree are also included elsewhere and that things go bad if those
>   changes get rebased after all, or won't make it into the merge window
>   for some reason.

Please never pull anything from me unless there's a signed tag for it,
anything else I won't know about and may be rebased or vanish or
something.

> I have taken Mark on Cc to let him know about the dependency now, and I've
> merged ab6f6d85210c4d0265cf48e9958c04e08595055a (which has only shmobile
> specific ASoC patches) into the next/boards2 branch before merging your
> branch. This is still not perfect because it breaks bisection, but it's
> the best I could do aside from forcing you do do another round-trip.

What's the bisection problem?
Arnd Bergmann - Nov. 13, 2012, 8:48 a.m.
On Tuesday 13 November 2012, Mark Brown wrote:
>   Show Details
>   On Mon, Nov 12, 2012 at 09:11:03PM +0000, Arnd Bergmann wrote:
> 
> > * Always let the person that owns the dependency know that the changes in
> >   their tree are also included elsewhere and that things go bad if those
> >   changes get rebased after all, or won't make it into the merge window
> >   for some reason.
> 
> Please never pull anything from me unless there's a signed tag for it,
> anything else I won't know about and may be rebased or vanish or
> something.

Ok, thanks for the reminder. Do you have a signed tag containing ab6f6d8521?

> > I have taken Mark on Cc to let him know about the dependency now, and I've
> > merged ab6f6d85210c4d0265cf48e9958c04e08595055a (which has only shmobile
> > specific ASoC patches) into the next/boards2 branch before merging your
> > branch. This is still not perfect because it breaks bisection, but it's
> > the best I could do aside from forcing you do do another round-trip.
> 
> What's the bisection problem?

I merged ab6f6d8521 first, and then Simon's branch, which means that the
linear (first-parent) history is ok, but bisecting the entire state of
the tree may end up in a commit in his branch in a state before it was
merged into mine, where it doesn't contain the patches it depends on,
and as Simon mentioned in his pull request, that will cause a build
failure.

	Arnd
Mark Brown - Nov. 13, 2012, 10:01 a.m.
On Tue, Nov 13, 2012 at 08:48:03AM +0000, Arnd Bergmann wrote:
> On Tuesday 13 November 2012, Mark Brown wrote:

> > Please never pull anything from me unless there's a signed tag for it,
> > anything else I won't know about and may be rebased or vanish or
> > something.

> Ok, thanks for the reminder. Do you have a signed tag containing ab6f6d8521?

Nope, your e-mail was the first I'd heard of the cross-merge.  I guess I
could create one if this is going to get redone for some reason...

> > > I have taken Mark on Cc to let him know about the dependency now, and I've
> > > merged ab6f6d85210c4d0265cf48e9958c04e08595055a (which has only shmobile
> > > specific ASoC patches) into the next/boards2 branch before merging your
> > > branch. This is still not perfect because it breaks bisection, but it's
> > > the best I could do aside from forcing you do do another round-trip.

> > What's the bisection problem?

> I merged ab6f6d8521 first, and then Simon's branch, which means that the
> linear (first-parent) history is ok, but bisecting the entire state of
> the tree may end up in a commit in his branch in a state before it was
> merged into mine, where it doesn't contain the patches it depends on,
> and as Simon mentioned in his pull request, that will cause a build
> failure.

Oh, I'd missed the fact that this wasn't coming in via a merge into
Simon's tree.
Arnd Bergmann - Nov. 13, 2012, 10:27 a.m.
On Tuesday 13 November 2012, Mark Brown wrote:
> On Tue, Nov 13, 2012 at 08:48:03AM +0000, Arnd Bergmann wrote:
> > On Tuesday 13 November 2012, Mark Brown wrote:
> 
> > > Please never pull anything from me unless there's a signed tag for it,
> > > anything else I won't know about and may be rebased or vanish or
> > > something.
> 
> > Ok, thanks for the reminder. Do you have a signed tag containing ab6f6d8521?
> 
> Nope, your e-mail was the first I'd heard of the cross-merge.  I guess I
> could create one if this is going to get redone for some reason...

I'm not planning to pull this one into somewhere else, just want to make
sure that you remeber it's been pulled into arm-soc. You mentioned above
that having a tag helps you with your bookkeeping.

	Arnd
Mark Brown - Nov. 14, 2012, 2:15 a.m.
On Tue, Nov 13, 2012 at 10:27:10AM +0000, Arnd Bergmann wrote:

> I'm not planning to pull this one into somewhere else, just want to make
> sure that you remeber it's been pulled into arm-soc. You mentioned above
> that having a tag helps you with your bookkeeping.

It's not that it's particularly helpful for my bookkeeping (I rarely if
ever look at the tags), it's more that if I've not signed a tag then it
means that I'm either not aware of or not OK with the branch being used.
Sergei Shtylyov - March 13, 2013, 9:45 p.m.
Hello.

On 11/09/2012 10:31 AM, Simon Horman wrote:

> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> This patch supports CN21/CN22 USB 2.0 (port 0/1/2),
> and enable USB momery on defconfig

    Sorry for late comment but I'm studying this code just now (in order 
to add R-Car M1A USB support).

> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> ---
>   arch/arm/configs/marzen_defconfig     |    6 ++
>   arch/arm/mach-shmobile/Kconfig        |    1 +
>   arch/arm/mach-shmobile/board-marzen.c |  108 ++++++++++++++++++++++++++++++++-
>   3 files changed, 114 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-shmobile/board-marzen.c b/arch/arm/mach-shmobile/board-marzen.c
> index 74c7f0b..707b3bd 100644
> --- a/arch/arm/mach-shmobile/board-marzen.c
> +++ b/arch/arm/mach-shmobile/board-marzen.c
> @@ -34,6 +34,9 @@
[...]
> @@ -172,6 +175,101 @@ static struct platform_device *marzen_devices[] __initdata = {
>   	&usb_phy_device,
>   };
>   
> +/* USB */
> +static struct usb_phy *phy;
> +static int usb_power_on(struct platform_device *pdev)
> +{
> +	if (!phy)
> +		return -EIO;
> +
> +	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_get_sync(&pdev->dev);
> +
> +	usb_phy_init(phy);
> +
> +	return 0;
> +}
> +
> +static void usb_power_off(struct platform_device *pdev)
> +{
> +	if (!phy)
> +		return;
> +
> +	usb_phy_shutdown(phy);
> +
> +	pm_runtime_put_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +}
> +
> +static struct usb_ehci_pdata ehcix_pdata = {
> +	.power_on	= usb_power_on,
> +	.power_off	= usb_power_off,
> +	.power_suspend	= usb_power_off,
> +};
> +
> +static struct resource ehci0_resources[] = {
> +	[0] = {
> +		.start	= 0xffe70000,
> +		.end	= 0xffe70400 - 1,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +	[1] = {
> +		.start	= gic_spi(44),
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +};
> +
> +static struct platform_device ehci0_device = {
> +	.name	= "ehci-platform",
> +	.id	= 0,
> +	.dev	= {
> +		.dma_mask		= &ehci0_device.dev.coherent_dma_mask,
> +		.coherent_dma_mask	= 0xffffffff,
> +		.platform_data		= &ehcix_pdata,
> +	},
> +	.num_resources	= ARRAY_SIZE(ehci0_resources),
> +	.resource	= ehci0_resources,
> +};
> +
> +static struct resource ehci1_resources[] = {
> +	[0] = {
> +		.start	= 0xfff70000,
> +		.end	= 0xfff70400 - 1,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +	[1] = {
> +		.start	= gic_spi(45),
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +};
> +
> +static struct platform_device ehci1_device = {
> +	.name	= "ehci-platform",
> +	.id	= 1,
> +	.dev	= {
> +		.dma_mask		= &ehci1_device.dev.coherent_dma_mask,
> +		.coherent_dma_mask	= 0xffffffff,
> +		.platform_data		= &ehcix_pdata,
> +	},
> +	.num_resources	= ARRAY_SIZE(ehci1_resources),
> +	.resource	= ehci1_resources,
> +};
> +
> +static struct platform_device *marzen_late_devices[] __initdata = {
> +	&ehci0_device,
> +	&ehci1_device,
> +};
> +
> +void __init marzen_init_late(void)
> +{
> +	/* get usb phy */
> +	phy = usb_get_phy(USB_PHY_TYPE_USB2);
> +
> +	shmobile_init_late();
> +	platform_add_devices(marzen_late_devices,
> +			     ARRAY_SIZE(marzen_late_devices));
> +}
> +
>

    Morimoto-san, I don't understand why this SoC specific platform device
ended up in the board file? Could you explain your reasons please?
    I think this is generally a bad practice as this approach scales badly.

WVR, Sergei
Sergei Shtylyov - March 13, 2013, 10:20 p.m.
Hello.

On 11/09/2012 10:31 AM, Simon Horman wrote:

> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> ---
>   arch/arm/configs/marzen_defconfig     |    3 ++-
>   arch/arm/mach-shmobile/board-marzen.c |   21 +++++++++++++++++++++
>   2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-shmobile/board-marzen.c b/arch/arm/mach-shmobile/board-marzen.c
> index 69f7f46..74c7f0b 100644
> --- a/arch/arm/mach-shmobile/board-marzen.c
> +++ b/arch/arm/mach-shmobile/board-marzen.c
> @@ -144,11 +144,32 @@ static struct platform_device hspi_device = {
>   	.num_resources	= ARRAY_SIZE(hspi_resources),
>   };
>   
> +/* USB PHY */
> +static struct resource usb_phy_resources[] = {
> +	[0] = {
> +		.start		= 0xffe70000,

    Why not 0xffe70800? 0xffe70000 is where the EHCI registers start.
I don't see why they should be overlapped like this.

> +		.end		= 0xffe70900 - 1,
> +		.flags		= IORESOURCE_MEM,
> +	},
> +	[1] = {
> +		.start		= 0xfff70000,
> +		.end		= 0xfff70900 - 1,

    Hm, this PHY register range doesn't seem to be documented?

> +		.flags		= IORESOURCE_MEM,
> +	},
> +};
> +
> +static struct platform_device usb_phy_device = {
> +	.name		= "rcar_usb_phy",
> +	.resource	= usb_phy_resources,
> +	.num_resources	= ARRAY_SIZE(usb_phy_resources),
> +};
> +
>   static struct platform_device *marzen_devices[] __initdata = {
>   	&eth_device,
>   	&sdhi0_device,
>   	&thermal_device,
>   	&hspi_device,
> +	&usb_phy_device,
>   };
>   
>   static void __init marzen_init(void)

    Finally, the same question as for EHCI: why this ended up in the 
board file instead of setup-r8a7779.c?

WBR, Sergei
Kuninori Morimoto - March 14, 2013, 12:29 a.m.
Hi Sergei

> > +/* USB */
> > +static struct usb_phy *phy;
> > +static int usb_power_on(struct platform_device *pdev)
> > +{
> > +	if (!phy)
> > +		return -EIO;
> > +
> > +	pm_runtime_enable(&pdev->dev);
> > +	pm_runtime_get_sync(&pdev->dev);
> > +
> > +	usb_phy_init(phy);
> > +
> > +	return 0;
> > +}
> > +
> > +static void usb_power_off(struct platform_device *pdev)
> > +{
> > +	if (!phy)
> > +		return;
> > +
> > +	usb_phy_shutdown(phy);
> > +
> > +	pm_runtime_put_sync(&pdev->dev);
> > +	pm_runtime_disable(&pdev->dev);
> > +}
> > +
> > +static struct usb_ehci_pdata ehcix_pdata = {
> > +	.power_on	= usb_power_on,
> > +	.power_off	= usb_power_off,
> > +	.power_suspend	= usb_power_off,
> > +};
(snip)
>     Morimoto-san, I don't understand why this SoC specific platform device
> ended up in the board file? Could you explain your reasons please?
>     I think this is generally a bad practice as this approach scales badly.

Do you mean it should exist in setup-r8a7779.c ?

I forgot detail of it, but these usb is using callback function,
and it is using *phy*.
This phy came from marzen_init_late() with usb_get_phy(USB_PHY_TYPE_USB2);
This usb_get_phy() is not needed if board doesn't have USB.

You can modify it if you want

Best regards
---
Kuninori Morimoto
Kuninori Morimoto - March 14, 2013, 12:44 a.m.
Hi Sergei

> > +/* USB PHY */
> > +static struct resource usb_phy_resources[] = {
> > +	[0] = {
> > +		.start		= 0xffe70000,
> 
>     Why not 0xffe70800? 0xffe70000 is where the EHCI registers start.
> I don't see why they should be overlapped like this.

Current rcar-phy try to access 0xffe7009x

> > +	[1] = {
> > +		.start		= 0xfff70000,
> > +		.end		= 0xfff70900 - 1,
> 
>     Hm, this PHY register range doesn't seem to be documented?

0xffe70000 is for port0/1
0xfff70000 is for port2

But this 0xfff70000 is only for 0xfff7009x access

Best regards
---
Kuninori Morimoto
Kuninori Morimoto - March 14, 2013, 1:09 a.m.
Hi Sergei

> >     Morimoto-san, I don't understand why this SoC specific platform device
> > ended up in the board file? Could you explain your reasons please?
> >     I think this is generally a bad practice as this approach scales badly.
> 
> Do you mean it should exist in setup-r8a7779.c ?
> 
> I forgot detail of it, but these usb is using callback function,
> and it is using *phy*.
> This phy came from marzen_init_late() with usb_get_phy(USB_PHY_TYPE_USB2);
> This usb_get_phy() is not needed if board doesn't have USB.
> 
> You can modify it if you want

And, as you pointed in USB ML,
This rcar_usb_phy driver will need board specific initialization.
(it is not implemented at this point...)


Best regards
---
Kuninori Morimoto
Sergei Shtylyov - March 14, 2013, 1:20 p.m.
Hello.

On 14-03-2013 4:44, Kuninori Morimoto wrote:

>>> +/* USB PHY */
>>> +static struct resource usb_phy_resources[] = {
>>> +	[0] = {
>>> +		.start		= 0xffe70000,

>>      Why not 0xffe70800? 0xffe70000 is where the EHCI registers start.
>> I don't see why they should be overlapped like this.

> Current rcar-phy try to access 0xffe7009x

    That should be changed too IHO, as these registers are not really PHY 
related, they control EHCI itself. These writes should be done somewhere in 
setup-r8a7779.c probably.

>>> +	[1] = {
>>> +		.start		= 0xfff70000,
>>> +		.end		= 0xfff70900 - 1,

>>      Hm, this PHY register range doesn't seem to be documented?

> 0xffe70000 is for port0/1
> 0xfff70000 is for port2

> But this 0xfff70000 is only for 0xfff7009x access

    Yeah, I understood that it's only for the extra EHCI registers.

> Best regards
> ---
> Kuninori Morimoto

WBR, Sergei
Sergei Shtylyov - March 14, 2013, 1:42 p.m.
Hello.

On 14-03-2013 4:29, Kuninori Morimoto wrote:

>>> +/* USB */
>>> +static struct usb_phy *phy;
>>> +static int usb_power_on(struct platform_device *pdev)
>>> +{
>>> +	if (!phy)
>>> +		return -EIO;
>>> +
>>> +	pm_runtime_enable(&pdev->dev);
>>> +	pm_runtime_get_sync(&pdev->dev);
>>> +
>>> +	usb_phy_init(phy);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void usb_power_off(struct platform_device *pdev)
>>> +{
>>> +	if (!phy)
>>> +		return;
>>> +
>>> +	usb_phy_shutdown(phy);
>>> +
>>> +	pm_runtime_put_sync(&pdev->dev);
>>> +	pm_runtime_disable(&pdev->dev);
>>> +}
>>> +
>>> +static struct usb_ehci_pdata ehcix_pdata = {
>>> +	.power_on	= usb_power_on,
>>> +	.power_off	= usb_power_off,
>>> +	.power_suspend	= usb_power_off,
>>> +};
> (snip)

>>      Morimoto-san, I don't understand why this SoC specific platform device
>> ended up in the board file? Could you explain your reasons please?
>>      I think this is generally a bad practice as this approach scales badly.

> Do you mean it should exist in setup-r8a7779.c ?

    Yes.

> I forgot detail of it, but these usb is using callback function,
> and it is using *phy*.

   But this PHY also belongs to SoC.

> This phy came from marzen_init_late() with usb_get_phy(USB_PHY_TYPE_USB2);
> This usb_get_phy() is not needed if board doesn't have USB.

    Anyway, there should be ways to separate the board specific platform code 
and the SoC specific one. That's what other subarches do.

> You can modify it if you want

    Yes, I definitely would like to try.

WBR, Sergei
Kuninori Morimoto - March 15, 2013, 12:52 a.m.
Hi Sergei

> > I forgot detail of it, but these usb is using callback function,
> > and it is using *phy*.
> 
>    But this PHY also belongs to SoC.
> 
> > This phy came from marzen_init_late() with usb_get_phy(USB_PHY_TYPE_USB2);
> > This usb_get_phy() is not needed if board doesn't have USB.
> 
>     Anyway, there should be ways to separate the board specific platform code 
> and the SoC specific one. That's what other subarches do.
>
> > You can modify it if you want
> 
>     Yes, I definitely would like to try.

I checked CPG :: MSTP for USB0/1/2 and USB Host/Function,
and GIC number for it.
It seems no conflict each other
I guess EHCI/OHCI goes to setup seems no problem.

But, as you pointed on USB ML,
USB-PHY :: USBPCTRL0 depends on platform
(Host/Function selection, and OVC pin setting)

rcar_usb_phy driver belongs to platform


Best regards
---
Kuninori Morimoto
Sergei Shtylyov - March 15, 2013, 12:51 p.m.
Hello.

On 15-03-2013 4:52, Kuninori Morimoto wrote:

>>> I forgot detail of it, but these usb is using callback function,
>>> and it is using *phy*.

>>     But this PHY also belongs to SoC.

>>> This phy came from marzen_init_late() with usb_get_phy(USB_PHY_TYPE_USB2);
>>> This usb_get_phy() is not needed if board doesn't have USB.

>>      Anyway, there should be ways to separate the board specific platform code
>> and the SoC specific one. That's what other subarches do.

>>> You can modify it if you want

>>      Yes, I definitely would like to try.

> I checked CPG :: MSTP for USB0/1/2 and USB Host/Function,
> and GIC number for it.
> It seems no conflict each other
> I guess EHCI/OHCI goes to setup seems no problem.

> But, as you pointed on USB ML,
> USB-PHY :: USBPCTRL0 depends on platform
> (Host/Function selection, and OVC pin setting)

> rcar_usb_phy driver belongs to platform

    No, it doesn't. Only it's (future) platform data will belong to the board --
that's what they were designed for.

WBR, Sergei