On 22 March 2016 at 08:44, Jun Nie [off-list ref] wrote:

2016-03-22 16:20 GMT+08:00 G Gregory [off-list ref]:

quoted

On 22 March 2016 at 02:17, Jun Nie [off-list ref] wrote:

quoted

2016-03-22 10:06 GMT+08:00 Timur Tabi [off-list ref]:

quoted

Jun Nie wrote:

quoted

That's a fair comment. I suppose all platform need to enable clock for
the device. For device tree case, we need these code for clock
enabling.

But why do you need to enable the clock?  The pl011 driver works fine for
every platform without your patch.  If the ZTE platform needs to call
clk_get() in order to work, than that is something different about your
platform.

ARM/ST pl011 is amba device and use pl011_probe() for initialization,
which call these clock enabling code for every device. You introduced
platform device driver entry for pl011 and only sbsa use platform
device probe method currently. I am trying to reuse this probe
function, so need to add clock enabling code in this function.

If you did not find any issue in sbsa device tree case, I guess the
clock is always enabled. For mobile platform, clock is auto gated for
power saving in most of cases.

quoted

quoted

For ACPI case, it may be handled automatically. If you agree
my opinion, I will move these lines to patch 1. Please correct me if I
am wrong.

I think you need to create a separate patch that adds that call, and make
sure it's tested by other platforms before it gets accepted.

These code only impact sbsa device as only sbsa device is initialized
via platform driver entry. So only your flavor is need for testing the
code. Thanks! :-)

In SBSA mode the UART clock is always enabled by the firmware and
there are no baud rate registers so we don't need to know the value of
the clock. In ACPI there is no method to represent the clock. So if
you add lines to unconditionally require a clock you will break SBSA
mode in ACPI.

Graeme

Right, Timur also already point out breakage of ACPI case. So I plan
to move clocking enabling code to device tree case handling. Do you
still have any other concern?

No, I was just making sure ACPI case was clear because it didn't look
so from earlier comments.

Graeme