Hi Alan
a) Register access is one of ipc commands, and we do provide more generic APIs
based on regmap subsystem in other patch,
with that, there will be not necessary to check platform(Broxton) for
register access.
b) we separate scu and pmc ipc in two drivers, is because they are many
difference in hw itself, it composite several sub devices
in hardware layer, and supported ipc commands are different, I expected user
driver will not check platform(Broxston...) to do
ipc with scu or pmc when no common commands.
Best wishes
Qipeng
-----Original Message-----
From: One Thousand Gnomes [mailto:[email protected]]
Sent: Tuesday, April 21, 2015 7:59 PM
To: Zha, Qipeng
Cc: [email protected]; [email protected]; Yang, Fei;
Zhong, Huiquan; Chen, Jason CJ; Zheng, Qi
Subject: Re: [PATCH] platform:x86: add Intel Broxton PMC IPC driver
On Tue, 21 Apr 2015 00:59:35 +0000
"Zha, Qipeng" <[email protected]> wrote:
> Hi Alan
>
> Fei, owner of intel_scu_ipc.c, suggested separate these two, since
> they are different hw, Not prefer to manage them in one driver.
They don't look that different to me except that it lacks some of the cleanups
as well as the naming to make it more Linxulike
(intel_scu_ipc_iowrite16 etc) that were done to the scu_ipc code.
What we really want to avoid is code elsewhere that ends up doing
if (broxton)
intel_pmc_ipc_foo(x)
else
intel_scu_ipc_foo(x);
I guess to judge that really means seeing who uses these routines. At the
moment the user you've posted takes the existing not very pretty API and wraps
it up again in another one with no obvious reason for all the layering, but it
does at least then provide the upper layer as a sort of abstraction.
For the code itself
- there are quite a few things that could be const
- you keep the register bases in both the platform resources of the
subdevices and directly, yet don't seem to use them directly ?
Alan
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86"
in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
