On 2018-01-25 01:05, Andy Shevchenko wrote:
On Thu, 2018-01-25 at 00:06 +0800, Haiyue Wang wrote:
The KCS (Keyboard Controller Style) interface is used to perform in-
band
IPMI communication between a server host and its BMC (BaseBoard
Management
Controllers).
+config ASPEED_KCS_IPMI_BMC
+ depends on ARCH_ASPEED || COMPILE_TEST
+ depends on IPMI_KCS_BMC
+ select REGMAP_MMIO
+ tristate "Aspeed KCS IPMI BMC driver"
+ help
+ Provides a driver for the KCS (Keyboard Controller Style)
IPMI
+ interface found on Aspeed SOCs (AST2400 and AST2500).
+
+ The driver implements the BMC side of the KCS contorller,
it
+ provides the access of KCS IO space for BMC side.
+static inline u8 read_data(struct kcs_bmc *kcs_bmc)
+{
+ return kcs_bmc->io_inputb(kcs_bmc, kcs_bmc->ioreg.idr);
+}
+
+static inline void write_data(struct kcs_bmc *kcs_bmc, u8 data)
+{
+ kcs_bmc->io_outputb(kcs_bmc, kcs_bmc->ioreg.odr, data);
+}
+
+static inline u8 read_status(struct kcs_bmc *kcs_bmc)
+{
+ return kcs_bmc->io_inputb(kcs_bmc, kcs_bmc->ioreg.str);
+}
+
+static inline void write_status(struct kcs_bmc *kcs_bmc, u8 data)
+{
+ kcs_bmc->io_outputb(kcs_bmc, kcs_bmc->ioreg.str, data);
+}
+
+static void update_status_bits(struct kcs_bmc *kcs_bmc, u8 mask, u8
val)
+{
+ u8 tmp;
+
+ tmp = read_status(kcs_bmc);
+
+ tmp &= ~mask;
+ tmp |= val & mask;
+
+ write_status(kcs_bmc, tmp);
+}
Shouldn't be above some kind of regmap API?
It is KCS spec defined IO access for hidden the low level, if the low
level supports regmap, such as in kcs_bmc_aspeed.c,
aspeed_kcs_inb & aspeed_kcs_outb.
+/* Different phases of the KCS BMC module */
+enum kcs_phases {
+ /* BMC should not be expecting nor sending any data. */
+ KCS_PHASE_IDLE,
Perhaps kernel-doc?
Code + inline comments should be better than kernel-doc ? Or move it out
like :
/* The interface for checksum offload between the stack and networking
drivers
* is as follows...
*
* A. IP checksum related features
*
* Drivers advertise checksum offload capabilities in the features of a
device.
* From the stack's point of view these are capabilities offered by the
driver,
* a driver typically only advertises features that it is capable of
offloading
* to its device.
*
* The checksum related features are:
*
* NETIF_F_HW_CSUM - The driver (or its device) is able to
compute one
* IP (one's complement) checksum for any combination
* of protocols or protocol layering. The checksum is
* computed and set in a packet per the CHECKSUM_PARTIAL
* interface (see below).
*
* NETIF_F_IP_CSUM - Driver (device) is only able to checksum plain
* TCP or UDP packets over IPv4. These are specifically
* unencapsulated packets of the form IPv4|TCP or
* IPv4|UDP where the Protocol field in the IPv4 header
* is TCP or UDP. The IPv4 header may contain IP options
* This feature cannot be set in features for a device
* with NETIF_F_HW_CSUM also set. This feature is being
* DEPRECATED (see below).
+};
+/* IPMI 2.0 - 9.5, KCS Interface Registers */
+struct kcs_ioreg {
+ u32 idr; /* Input Data Register */
+ u32 odr; /* Output Data Register */
+ u32 str; /* Status Register */
kernel-doc
+};
+
+static inline void *kcs_bmc_priv(const struct kcs_bmc *kcs_bmc)
+{
+ return kcs_bmc->priv;
+}
+
+extern int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc);
+extern struct kcs_bmc *kcs_bmc_alloc(struct device *dev, int
sizeof_priv,
+ u32 channel);
Drop extern.
After dropping extern, it truly passed compilation, have any special
reason to drop 'extern' ?
I saw in kernel still use extern like : extern void printk_nmi_init(void);
+#endif
Next one could be reviewed when you split this patch to two.
Got it!
