On Tue, Jul 14, 2015 at 5:46 PM, James Liao [off-list ref] wrote:

Hi Daniel,

On Tue, 2015-07-14 at 11:23 +0800, Daniel Kurtz wrote:

quoted

On Tue, Jul 14, 2015 at 10:45 AM, James Liao [off-list ref] wrote:

quoted

On Mon, 2015-07-13 at 22:46 +0800, Daniel Kurtz wrote:

quoted

quoted

+static const struct clk_ops mtk_ref2usb_tx_ops = {
+       .is_prepared    = mtk_ref2usb_tx_is_prepared,
+       .prepare        = mtk_ref2usb_tx_prepare,
+       .unprepare      = mtk_ref2usb_tx_unprepare,
+};

Burying the implementation of this special "mtk_ref2usb" clock in
clk-mt8173,c seems a bit awkward.
Can you please move it to its own file, like mediatek/clk-usb.c?

Do you mean clk/mediatek/clk-usb.c?

This clock is a MT8173 specific clock, which may not be reused by other
SoCs. So I think it's not necessary to move it to a separated file.

Yes, this is a wrapper for a type of clock.  I think it would be
better in its own file, rather than embedded in clk-mt8173.c, which is
more about enumerating and initializing the clock tree.

Perhaps today it is only used for mt8173, but that may not be the case
in the future.

OK, I'll separate it from clk-mt8173.c.

Ref2usb_tx's control register is located in APMIXEDSYS, which contains
most of PLL controlling. Is this clock suitable to implemented
mtk-pll.c? Or it's proper to be a separated file such as clk-usb.c?

I think a separate clk-usb.c might be better since the driver is very
different than what is used for the PLLs.

Thanks!
-Dan

Best regards,

James