Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1283776

Re: [PATCH v3 03/18] clocksource: Add NPS400 timers driver

Show all headers | View raw

>From: Daniel Lezcano <daniel.lezcano@linaro.org>
>Sent: Friday, December 4, 2015 11:13 AM
>> +obj-$(CONFIG_ARC_PLAT_EZNPS) += timer-nps.o

>CONFIG_CLKSRC_NPS
I wish this driver to be build only for this specific ARC platform.
This clock source is embedded in our SoC.
It is not meant to be built for any other architecture.
This is why below I include header from our ARC platform to avoid code duplicity.

>> +#include <plat/ctop.h>

>Why do you need this header ? nps_host_reg ?
Correct we use common code from our platform.

>We prevent to include headers from <plat> in the drivers directory. You
>should find a way to get rid of it.

The Only way I can think of is code duplicity and I prefer not to.
I see some include to asm/mach headers in clocksource, what is the difference?
Could you suggest a better place for me to place my header file.

>> +static void *nps_msu_reg_low_addr[NPS_CLUSTER_NUM] __read_mostly;
>
>Perhaps a small optimization...
Thanks

>static DEFINE_PER_CPU_READ_MOSTLY(void __iomem *, baseaddr);

>static cycle_t nps_clksrc_read(struct clocksource *clksrc)
>{
>       void __iomem *base = per_cpu(baseaddr, raw_smp_processor_id());
>
 >       return (cycle_t)ioread32be(base);
>}

>and in the init function:

>for_each_cpu(cpu) {
>       per_cpu(baseaddr, cpu) = nps_host_reg(cpu,
>                                      NPS_MSU_BLKID,
>                                    NPS_MSU_TICK_LOW
>}
Thanks again


>> +static cycle_t nps_clksrc_read(struct clocksource *clksrc)
>> +{
>> +     int cluster = raw_smp_processor_id() >> NPS_CLUSTER_OFFSET;
>> +
>> +     return (cycle_t)ioread32be(nps_msu_reg_low_addr[cluster]);

>,AFAICT, there is a memory barrier with ioread32be, are you really sure
>we have to use it in this code path ?
Are you saying to remove use of ioread32be?
What should I use instead?

>> +
>> +     dt_root = of_get_flat_dt_root();
>> +     rate = (u32)of_get_flat_dt_prop(dt_root, "clock-frequency", NULL);

>I don't get why this is done this way. The Kconfig option help says the
>clocksource rate is 1GHz but in the DT the clock is 88MHz.
It says that clock source is up to 1GHz

>It would be cleaner to define a fixed clock and then add a phandle in
>the DT.

>        timer_clk: timer_clk {
>               #clock-cells = <0>;
>                compatible = "fixed-clock";
>               clock-frequency = <123456789>;
>        };

>        timer {
>               compatible = "ezchip,nps400-timer";
>                clocks = <&timer_clk>;
>       }

>That will result in the same code than the other drivers.

>      clk = of_clk_get(np, 0);
>     if (IS_ERR(clk)) {
>              pr_err("%s: invalid clock\n", np->full_name);
>               return;
>        }

>      rate = clk_get_rate(clk);
Once again thanks I will fix this.

-Noam
--
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/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v3 00/18] *** SUBJECT HERE *** Noam Camus <noamc@ezchip.com> - 2015-12-01 14:10 +0100
  Re: [PATCH v3 04/18] irqchip: add nps Internal and external irqchips Marc Zyngier <marc.zyngier@arm.com> - 2015-12-01 14:30 +0100
    RE: [PATCH v3 04/18] irqchip: add nps Internal and external irqchips Noam Camus <noamc@ezchip.com> - 2015-12-02 16:10 +0100
      Re: [PATCH v3 04/18] irqchip: add nps Internal and external irqchips Marc Zyngier <marc.zyngier@arm.com> - 2015-12-03 19:40 +0100
        RE: [PATCH v3 04/18] irqchip: add nps Internal and external irqchips Noam Camus <noamc@ezchip.com> - 2015-12-07 12:20 +0100
    Re: [PATCH v3 04/18] irqchip: add nps Internal and external irqchips Vineet Gupta <Vineet.Gupta1@synopsys.com> - 2015-12-11 09:00 +0100
  Re: [PATCH v3 01/18] Documentation: Add EZchip vendor to binding list Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-12-04 09:10 +0100
    Re: [PATCH v3 01/18] Documentation: Add EZchip vendor to binding list Noam Camus <noamc@ezchip.com> - 2015-12-04 12:50 +0100
  Re: [PATCH v3 03/18] clocksource: Add NPS400 timers driver Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-12-04 10:20 +0100
    Re: [PATCH v3 03/18] clocksource: Add NPS400 timers driver Noam Camus <noamc@ezchip.com> - 2015-12-04 13:30 +0100
      Re: [PATCH v3 03/18] clocksource: Add NPS400 timers driver Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-12-04 14:00 +0100
    RE: [PATCH v3 03/18] clocksource: Add NPS400 timers driver Noam Camus <noamc@ezchip.com> - 2015-12-08 14:10 +0100

csiph-web