[U-Boot] [PATCH v2 02/13] drivers: clk: Add clock driver for Microchip PIC32 Microcontroller.

[Purna Chandra Mandal]

On 01/11/2016 10:27 PM, Simon Glass wrote:
> Hi,
>
> On 4 January 2016 at 07:00, Purna Chandra Mandal
> <purna.mandal at microchip.com> wrote:
>> Signed-off-by: Purna Chandra Mandal <purna.mandal at microchip.com>
>>
> Commit message please.

Ack. Will add.

>> ---
>>
>> Changes in v2:
>> - add get clock rate for mpll clock
>>
>>  .../clock/microchip,pic32-clock.txt                |  28 ++
>>  drivers/clk/Makefile                               |   1 +
>>  drivers/clk/clk-pic32.c                            | 427 +++++++++++++++++++++
>>  include/dt-bindings/clock/microchip,clock.h        |  29 ++
>>  4 files changed, 485 insertions(+)
>>  create mode 100644 doc/device-tree-bindings/clock/microchip,pic32-clock.txt
>>  create mode 100644 drivers/clk/clk-pic32.c
>>  create mode 100644 include/dt-bindings/clock/microchip,clock.h
>>
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> nits below
>
>> diff --git a/doc/device-tree-bindings/clock/microchip,pic32-clock.txt b/doc/device-tree-bindings/clock/microchip,pic32-clock.txt
>> new file mode 100644
>> index 0000000..d02b9d7
>> --- /dev/null
>> +++ b/doc/device-tree-bindings/clock/microchip,pic32-clock.txt
>> @@ -0,0 +1,28 @@
>> +* Microchip PIC32 Clock and Oscillator
>> +
>> +The PIC32 clock controller generates and supplies clock to various
>> +controllers within the SoC.
>> +
>> +Required Properties:
>> +
>> +- compatible: should be "microchip,pic32mzda_clk"
>> +- reg: physical base address of the controller and length of memory mapped
>> +  region.
>> +- #clock-cells: should be 1.
>> +
>> +Example: Clock controller node:
>> +
>> +       clock: clk at 1f801200 {
>> +               compatible = "microchip,pic32mzda_clk";
>> +               reg = <0xbf801200 0x1000>;
>> +       };
>> +
>> +Example: UART controller node that consumes the clock generated by the clock
>> +  controller:
>> +
>> +       uart1: serial at 1f822000 {
>> +               compatible = "microchip,pic32mzda-uart";
>> +               reg = <0xbf822000 0x50>;
>> +               interrupts = <112 IRQ_TYPE_LEVEL_HIGH>;
>> +               clocks = <&clock PB2CLK>;
>> +       };
>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>> index 4a6a4a8..3c84e08 100644
>> --- a/drivers/clk/Makefile
>> +++ b/drivers/clk/Makefile
>> @@ -9,3 +9,4 @@ obj-$(CONFIG_CLK) += clk-uclass.o
>>  obj-$(CONFIG_ROCKCHIP_RK3036) += clk_rk3036.o
>>  obj-$(CONFIG_ROCKCHIP_RK3288) += clk_rk3288.o
>>  obj-$(CONFIG_SANDBOX) += clk_sandbox.o
>> +obj-$(CONFIG_MACH_PIC32) += clk-pic32.o
> As the other review mentions, should use underscore in filenames
> unless it is a uclass.

Agreed.

>> diff --git a/drivers/clk/clk-pic32.c b/drivers/clk/clk-pic32.c
>> new file mode 100644
>> index 0000000..70aac05
>> --- /dev/null
>> +++ b/drivers/clk/clk-pic32.c
>> @@ -0,0 +1,427 @@
>> +/*
>> + * Copyright (C) 2015 Purna Chandra Mandal <purna.mandal at microchip.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + *
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <clk.h>
> nit: clk.h should go above dm.h, below common.h

ack. If you could share *specific* reason behind this order (like dependencies)?

>> +#include <div64.h>
>> +#include <wait_bit.h>
>> +#include <dm/lists.h>
>> +#include <asm/io.h>
>> +#include <mach/pic32.h>
>> +#include <dt-bindings/clock/microchip,clock.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
> Regards,
> Simon