Hi Ryder,
On 25 October 2018 at 00:37, Ryder Lee <ryder....@mediatek.com> wrote:
> On Wed, 2018-10-24 at 21:29 -0600, Simon Glass wrote:
>> Hi Ryder,
>>
>> On 12 October 2018 at 01:00, Ryder Lee <ryder....@mediatek.com> wrote:
>> > This patch adds clock modules for MediaTek SoCs:
>> > - Shared part: a common driver which contains the general operations
>> > for plls, muxes, dividers and gates so that we can reuse it in future.
>> >
>> > - Specific SoC part: the group of structures used to hold the hardware
>> > configuration for each SoC.
>> >
>> > We take MT7629 as an example to demonstrate how to implement driver if
>> > any other MediaTek chips would like to use it.
>> >
>> > Signed-off-by: Ryder Lee <ryder....@mediatek.com>
>> > ---
>> > drivers/clk/Makefile | 1 +
>> > drivers/clk/mediatek/Makefile | 6 +
>> > drivers/clk/mediatek/clk-mt7629.c | 709
>> > ++++++++++++++++++++++++++++++++++++++
>> > drivers/clk/mediatek/clk-mtk.c | 492 ++++++++++++++++++++++++++
>> > drivers/clk/mediatek/clk-mtk.h | 153 ++++++++
>> > 5 files changed, 1361 insertions(+)
>> > create mode 100644 drivers/clk/mediatek/Makefile
>> > create mode 100644 drivers/clk/mediatek/clk-mt7629.c
>> > create mode 100644 drivers/clk/mediatek/clk-mtk.c
>> > create mode 100644 drivers/clk/mediatek/clk-mtk.h
>>
>> Looks good except for a few things below.
>>
>> [..]
>>
>> > +const struct clk_ops mtk_clk_gate_ops = {
>> > + .enable = mtk_clk_gate_enable,
>> > + .disable = mtk_clk_gate_disable,
>> > + .get_rate = mtk_clk_gate_get_rate,
>> > +};
>> > +
>> > +int mtk_clk_init(struct udevice *dev, const struct mtk_clk_tree *tree)
>> > +{
>> > + struct mtk_clk_priv *priv = dev_get_priv(dev);
>> > +
>> > + priv->base = dev_read_addr_ptr(dev);
>> > + if (!priv->base)
>> > + return -ENOENT;
>>
>> Why do you export these two functions? Devices should be probed in the
>> normal DM way.
>
> Yes, they are probed in the normal way. These functions are used by
> several clock blocks and MTK SoCs. so I put the common parts here to
> keep the code as clean as possible.
OK I see, thank you. How about a _common suffix on the function to
make this clear?
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
