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:gnomes@xxxxxxxxxxxxxxxxxxx]
Sent: Tuesday, April 21, 2015 7:59 PM
To: Zha, Qipeng
Cc: platform-driver-x86@xxxxxxxxxxxxxxx; dvhart@xxxxxxxxxxxxx; 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" <qipeng.zha@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
