| Message ID | 1300184096-13937-2-git-send-email-jamie@jamieiles.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On 10:14 Tue 15 Mar , Jamie Iles wrote: > The macb driver expects clocks with the names "pclk" and "hclk". We > currently provide "macb_clk" but to fit in line with other > architectures (namely AVR32), provide "pclk" and a fake "hclk". > > Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> > Signed-off-by: Jamie Iles <jamie@jamieiles.com> > --- > arch/arm/mach-at91/at572d940hf.c | 8 +++++++- > arch/arm/mach-at91/at91cap9.c | 8 +++++++- > arch/arm/mach-at91/at91sam9260.c | 8 +++++++- > arch/arm/mach-at91/at91sam9263.c | 8 +++++++- > arch/arm/mach-at91/at91sam9g45.c | 8 +++++++- > 5 files changed, 35 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/mach-at91/at572d940hf.c b/arch/arm/mach-at91/at572d940hf.c > index a6b9c68..9b3a37e 100644 > --- a/arch/arm/mach-at91/at572d940hf.c > +++ b/arch/arm/mach-at91/at572d940hf.c > @@ -71,10 +71,15 @@ static struct clk pioC_clk = { > .type = CLK_TYPE_PERIPHERAL, > }; > static struct clk macb_clk = { > - .name = "macb_clk", > + .name = "pclk", > .pmc_mask = 1 << AT572D940HF_ID_EMAC, > .type = CLK_TYPE_PERIPHERAL, > }; > +static struct clk macb_hclk = { > + .name = "hclk", > + .pmc_mask = 0, > + .type = CLK_TYPE_PERIPHERAL, > +}; for the fake clock you must specify the parent as macb_clk take a look on the tcb1_clk for the 9g45 Best Regards, J. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 15, 2011 at 01:35:39PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 10:14 Tue 15 Mar , Jamie Iles wrote: > > The macb driver expects clocks with the names "pclk" and "hclk". We > > currently provide "macb_clk" but to fit in line with other > > architectures (namely AVR32), provide "pclk" and a fake "hclk". > > > > Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> > > Signed-off-by: Jamie Iles <jamie@jamieiles.com> > > --- > > arch/arm/mach-at91/at572d940hf.c | 8 +++++++- > > arch/arm/mach-at91/at91cap9.c | 8 +++++++- > > arch/arm/mach-at91/at91sam9260.c | 8 +++++++- > > arch/arm/mach-at91/at91sam9263.c | 8 +++++++- > > arch/arm/mach-at91/at91sam9g45.c | 8 +++++++- > > 5 files changed, 35 insertions(+), 5 deletions(-) > > > > diff --git a/arch/arm/mach-at91/at572d940hf.c b/arch/arm/mach-at91/at572d940hf.c > > index a6b9c68..9b3a37e 100644 > > --- a/arch/arm/mach-at91/at572d940hf.c > > +++ b/arch/arm/mach-at91/at572d940hf.c > > @@ -71,10 +71,15 @@ static struct clk pioC_clk = { > > .type = CLK_TYPE_PERIPHERAL, > > }; > > static struct clk macb_clk = { > > - .name = "macb_clk", > > + .name = "pclk", > > .pmc_mask = 1 << AT572D940HF_ID_EMAC, > > .type = CLK_TYPE_PERIPHERAL, > > }; > > +static struct clk macb_hclk = { > > + .name = "hclk", > > + .pmc_mask = 0, > > + .type = CLK_TYPE_PERIPHERAL, > > +}; > for the fake clock you must specify the parent as macb_clk > > take a look on the tcb1_clk for the 9g45 Ok, will do. Thanks for the pointer! Jamie -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
hi, On Tue, Mar 15, 2011 at 12:14 PM, Jamie Iles <jamie@jamieiles.com> wrote: > The macb driver expects clocks with the names "pclk" and "hclk". We > currently provide "macb_clk" but to fit in line with other > architectures (namely AVR32), provide "pclk" and a fake "hclk". There is no reference to a "pclk" or "hclk" in the AT91 architecture. So to avoid possible confusion, maybe create two "fake" clocks both parented to "macb_clk", and add a comment they're only for compatibility with the AVR32. Regards, Andrew Victor -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 16, 2011 at 08:53:47AM +0200, avictor.za@gmail.com wrote: > hi, > > On Tue, Mar 15, 2011 at 12:14 PM, Jamie Iles <jamie@jamieiles.com> wrote: > > The macb driver expects clocks with the names "pclk" and "hclk". We > > currently provide "macb_clk" but to fit in line with other > > architectures (namely AVR32), provide "pclk" and a fake "hclk". > > There is no reference to a "pclk" or "hclk" in the AT91 architecture. > So to avoid possible confusion, maybe create two "fake" clocks both > parented to "macb_clk", and add a comment they're only for > compatibility with the AVR32. It doesn't matter what's in the documentation. There's no apb_pclk mentioned in the ARM platform documentation yet we have such a clock in the bus-level code for primecell support. I'm sure many of the other early primecell using platforms are also the same. On OMAP1 there's no iclk yet OMAP drivers have had iclk and iclk is a dummy no-op clock for OMAP1 for compatibility with OMAP2. What matters more than conforming to documentation is keeping the drivers in a clean and maintainable state without throwing lots of ifdefs into them. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
hi Russell, >> There is no reference to a "pclk" or "hclk" in the AT91 architecture. >> So to avoid possible confusion, maybe create two "fake" clocks both >> parented to "macb_clk", and add a comment they're only for >> compatibility with the AVR32. > > It doesn't matter what's in the documentation. > > What matters more than conforming to documentation is keeping the drivers > in a clean and maintainable state without throwing lots of ifdefs into > them. I'm not saying the drivers need ifdefs, they should request both "pclk" and "hclk" as suggested. What I was suggesting is the platform clock setup on AT91 as: macb_clk | +-- hclk +-- pclk rather than: pclk | +-- hclk Regards, Andrew Victor -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 17, 2011 at 11:22:37AM +0200, Andrew Victor wrote: > hi Russell, > > >> There is no reference to a "pclk" or "hclk" in the AT91 architecture. > >> So to avoid possible confusion, maybe create two "fake" clocks both > >> parented to "macb_clk", and add a comment they're only for > >> compatibility with the AVR32. > > > > It doesn't matter what's in the documentation. > > > > What matters more than conforming to documentation is keeping the drivers > > in a clean and maintainable state without throwing lots of ifdefs into > > them. > > I'm not saying the drivers need ifdefs, they should request both > "pclk" and "hclk" as suggested. > > What I was suggesting is the platform clock setup on AT91 as: > macb_clk > | > +-- hclk > +-- pclk > > rather than: > pclk > | > +-- hclk And what I've been saying all along is to make pclk a _dummy_ clock on the platform it doesn't exist for, rather than making it related in some way to another clock given to the peripheral. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 17, 2011 at 10:00:10AM +0000, Russell King - ARM Linux wrote: > On Thu, Mar 17, 2011 at 11:22:37AM +0200, Andrew Victor wrote: > > hi Russell, > > > > >> There is no reference to a "pclk" or "hclk" in the AT91 architecture. > > >> So to avoid possible confusion, maybe create two "fake" clocks both > > >> parented to "macb_clk", and add a comment they're only for > > >> compatibility with the AVR32. > > > > > > It doesn't matter what's in the documentation. > > > > > > What matters more than conforming to documentation is keeping the drivers > > > in a clean and maintainable state without throwing lots of ifdefs into > > > them. > > > > I'm not saying the drivers need ifdefs, they should request both > > "pclk" and "hclk" as suggested. > > > > What I was suggesting is the platform clock setup on AT91 as: > > macb_clk > > | > > +-- hclk > > +-- pclk > > > > rather than: > > pclk > > | > > +-- hclk > > And what I've been saying all along is to make pclk a _dummy_ clock on > the platform it doesn't exist for, rather than making it related in some > way to another clock given to the peripheral. Ok, so just to summarize, before my patches, at91 provides "macb_clk", whereas avr32 provides "pclk" and "hclk". I've renamed at91's "macb_clk" to "pclk" and added a fake, unrelated "hclk". It was suggested that the fake "hclk" should be a child of "pclk" but you're saying to leave it as I have it right? Jamie -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-at91/at572d940hf.c b/arch/arm/mach-at91/at572d940hf.c index a6b9c68..9b3a37e 100644 --- a/arch/arm/mach-at91/at572d940hf.c +++ b/arch/arm/mach-at91/at572d940hf.c @@ -71,10 +71,15 @@ static struct clk pioC_clk = { .type = CLK_TYPE_PERIPHERAL, }; static struct clk macb_clk = { - .name = "macb_clk", + .name = "pclk", .pmc_mask = 1 << AT572D940HF_ID_EMAC, .type = CLK_TYPE_PERIPHERAL, }; +static struct clk macb_hclk = { + .name = "hclk", + .pmc_mask = 0, + .type = CLK_TYPE_PERIPHERAL, +}; static struct clk usart0_clk = { .name = "usart0_clk", .pmc_mask = 1 << AT572D940HF_ID_US0, @@ -182,6 +187,7 @@ static struct clk *periph_clocks[] __initdata = { &pioB_clk, &pioC_clk, &macb_clk, + &macb_hclk, &usart0_clk, &usart1_clk, &usart2_clk, diff --git a/arch/arm/mach-at91/at91cap9.c b/arch/arm/mach-at91/at91cap9.c index 7337617..0d38ce7 100644 --- a/arch/arm/mach-at91/at91cap9.c +++ b/arch/arm/mach-at91/at91cap9.c @@ -150,10 +150,15 @@ static struct clk pwm_clk = { .type = CLK_TYPE_PERIPHERAL, }; static struct clk macb_clk = { - .name = "macb_clk", + .name = "pclk", .pmc_mask = 1 << AT91CAP9_ID_EMAC, .type = CLK_TYPE_PERIPHERAL, }; +static struct clk macb_hclk = { + .name = "hclk", + .pmc_mask = 0, + .type = CLK_TYPE_PERIPHERAL, +}; static struct clk aestdes_clk = { .name = "aestdes_clk", .pmc_mask = 1 << AT91CAP9_ID_AESTDES, @@ -212,6 +217,7 @@ static struct clk *periph_clocks[] __initdata = { &tcb_clk, &pwm_clk, &macb_clk, + &macb_hclk, &aestdes_clk, &adc_clk, &isi_clk, diff --git a/arch/arm/mach-at91/at91sam9260.c b/arch/arm/mach-at91/at91sam9260.c index 195208b..f00774c 100644 --- a/arch/arm/mach-at91/at91sam9260.c +++ b/arch/arm/mach-at91/at91sam9260.c @@ -162,10 +162,15 @@ static struct clk ohci_clk = { .type = CLK_TYPE_PERIPHERAL, }; static struct clk macb_clk = { - .name = "macb_clk", + .name = "pclk", .pmc_mask = 1 << AT91SAM9260_ID_EMAC, .type = CLK_TYPE_PERIPHERAL, }; +static struct clk macb_hclk = { + .name = "hclk", + .pmc_mask = 0, + .type = CLK_TYPE_PERIPHERAL, +}; static struct clk isi_clk = { .name = "isi_clk", .pmc_mask = 1 << AT91SAM9260_ID_ISI, @@ -221,6 +226,7 @@ static struct clk *periph_clocks[] __initdata = { &tc2_clk, &ohci_clk, &macb_clk, + &macb_hclk, &isi_clk, &usart3_clk, &usart4_clk, diff --git a/arch/arm/mach-at91/at91sam9263.c b/arch/arm/mach-at91/at91sam9263.c index 249f900..25cbae1 100644 --- a/arch/arm/mach-at91/at91sam9263.c +++ b/arch/arm/mach-at91/at91sam9263.c @@ -136,10 +136,15 @@ static struct clk pwm_clk = { .type = CLK_TYPE_PERIPHERAL, }; static struct clk macb_clk = { - .name = "macb_clk", + .name = "pclk", .pmc_mask = 1 << AT91SAM9263_ID_EMAC, .type = CLK_TYPE_PERIPHERAL, }; +static struct clk macb_hclk = { + .name = "hclk", + .pmc_mask = 0, + .type = CLK_TYPE_PERIPHERAL, +}; static struct clk dma_clk = { .name = "dma_clk", .pmc_mask = 1 << AT91SAM9263_ID_DMA, @@ -190,6 +195,7 @@ static struct clk *periph_clocks[] __initdata = { &tcb_clk, &pwm_clk, &macb_clk, + &macb_hclk, &twodge_clk, &udc_clk, &isi_clk, diff --git a/arch/arm/mach-at91/at91sam9g45.c b/arch/arm/mach-at91/at91sam9g45.c index c67b47f..a4d4a2d 100644 --- a/arch/arm/mach-at91/at91sam9g45.c +++ b/arch/arm/mach-at91/at91sam9g45.c @@ -157,10 +157,15 @@ static struct clk ac97_clk = { .type = CLK_TYPE_PERIPHERAL, }; static struct clk macb_clk = { - .name = "macb_clk", + .name = "pclk", .pmc_mask = 1 << AT91SAM9G45_ID_EMAC, .type = CLK_TYPE_PERIPHERAL, }; +static struct clk macb_hclk = { + .name = "hclk", + .pmc_mask = 0, + .type = CLK_TYPE_PERIPHERAL, +}; static struct clk isi_clk = { .name = "isi_clk", .pmc_mask = 1 << AT91SAM9G45_ID_ISI, @@ -224,6 +229,7 @@ static struct clk *periph_clocks[] __initdata = { &lcdc_clk, &ac97_clk, &macb_clk, + &macb_hclk, &isi_clk, &udphs_clk, &mmc1_clk,
The macb driver expects clocks with the names "pclk" and "hclk". We currently provide "macb_clk" but to fit in line with other architectures (namely AVR32), provide "pclk" and a fake "hclk". Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> Signed-off-by: Jamie Iles <jamie@jamieiles.com> --- arch/arm/mach-at91/at572d940hf.c | 8 +++++++- arch/arm/mach-at91/at91cap9.c | 8 +++++++- arch/arm/mach-at91/at91sam9260.c | 8 +++++++- arch/arm/mach-at91/at91sam9263.c | 8 +++++++- arch/arm/mach-at91/at91sam9g45.c | 8 +++++++- 5 files changed, 35 insertions(+), 5 deletions(-)