linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [char-misc-next 1/3] mei: validate the message header only in first fragment.
@ 2017-06-12  9:15 Tomas Winkler
  2017-06-12  9:15 ` [char-misc-next 2/3] mei: drop unreachable code in mei_start Tomas Winkler
  2017-06-12  9:15 ` [char-misc-next 3/3] mei: me: use an index instead of a pointer for private data Tomas Winkler
  0 siblings, 2 replies; 5+ messages in thread
From: Tomas Winkler @ 2017-06-12  9:15 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Alexander Usyskin, linux-kernel, Tomas Winkler

RX message header is received in the first fragment of
the message and saved side and it is not modified after that,
we don't need to validate it upon each fragment.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/interrupt.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index c14e35201721..b0b8f18a85e3 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -235,6 +235,17 @@ static inline bool hdr_is_fixed(struct mei_msg_hdr *mei_hdr)
 	return mei_hdr->host_addr == 0 && mei_hdr->me_addr != 0;
 }
 
+static inline int hdr_is_valid(u32 msg_hdr)
+{
+	struct mei_msg_hdr *mei_hdr;
+
+	mei_hdr = (struct mei_msg_hdr *)&msg_hdr;
+	if (!msg_hdr || mei_hdr->reserved)
+		return -EBADMSG;
+
+	return 0;
+}
+
 /**
  * mei_irq_read_handler - bottom half read routine after ISR to
  * handle the read processing.
@@ -256,17 +267,18 @@ int mei_irq_read_handler(struct mei_device *dev,
 		dev->rd_msg_hdr = mei_read_hdr(dev);
 		(*slots)--;
 		dev_dbg(dev->dev, "slots =%08x.\n", *slots);
-	}
-	mei_hdr = (struct mei_msg_hdr *) &dev->rd_msg_hdr;
-	dev_dbg(dev->dev, MEI_HDR_FMT, MEI_HDR_PRM(mei_hdr));
 
-	if (mei_hdr->reserved || !dev->rd_msg_hdr) {
-		dev_err(dev->dev, "corrupted message header 0x%08X\n",
+		ret = hdr_is_valid(dev->rd_msg_hdr);
+		if (ret) {
+			dev_err(dev->dev, "corrupted message header 0x%08X\n",
 				dev->rd_msg_hdr);
-		ret = -EBADMSG;
-		goto end;
+			goto end;
+		}
 	}
 
+	mei_hdr = (struct mei_msg_hdr *)&dev->rd_msg_hdr;
+	dev_dbg(dev->dev, MEI_HDR_FMT, MEI_HDR_PRM(mei_hdr));
+
 	if (mei_slots2data(*slots) < mei_hdr->length) {
 		dev_err(dev->dev, "less data available than length=%08x.\n",
 				*slots);
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [char-misc-next 2/3] mei: drop unreachable code in mei_start
  2017-06-12  9:15 [char-misc-next 1/3] mei: validate the message header only in first fragment Tomas Winkler
@ 2017-06-12  9:15 ` Tomas Winkler
  2017-06-12  9:15 ` [char-misc-next 3/3] mei: me: use an index instead of a pointer for private data Tomas Winkler
  1 sibling, 0 replies; 5+ messages in thread
From: Tomas Winkler @ 2017-06-12  9:15 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Alexander Usyskin, linux-kernel, Tomas Winkler

From: Alexander Usyskin <alexander.usyskin@intel.com>

Device disabled state is caught inside the retry loop, so
there is no need to check it once again afterwards.

Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/init.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index c8ad9ee7cb80..d2f691424dd1 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -215,12 +215,6 @@ int mei_start(struct mei_device *dev)
 		}
 	} while (ret);
 
-	/* we cannot start the device w/o hbm start message completed */
-	if (dev->dev_state == MEI_DEV_DISABLED) {
-		dev_err(dev->dev, "reset failed");
-		goto err;
-	}
-
 	if (mei_hbm_start_wait(dev)) {
 		dev_err(dev->dev, "HBM haven't started");
 		goto err;
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [char-misc-next 3/3] mei: me: use an index instead of a pointer for private data
  2017-06-12  9:15 [char-misc-next 1/3] mei: validate the message header only in first fragment Tomas Winkler
  2017-06-12  9:15 ` [char-misc-next 2/3] mei: drop unreachable code in mei_start Tomas Winkler
@ 2017-06-12  9:15 ` Tomas Winkler
  2017-06-13 13:46   ` Greg Kroah-Hartman
  1 sibling, 1 reply; 5+ messages in thread
From: Tomas Winkler @ 2017-06-12  9:15 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Alexander Usyskin, linux-kernel, Tomas Winkler

Device 'new_id' interface is useful for testing of not yet published
hardware on older kernels and for internally used device ids on
simulation platforms.
However currently with the device configuration held in device_id driver
data as a pointer to mei_cfg structure it is hard, as one need to locate
the address of the correct structure.
A recommended way of doing that is to use and index instead of a
pointer.
This patch adds a new list of configuration mei_cfg_list[]
indexed via enum mei_cfg_idx.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/misc/mei/hw-me.c  |  32 +++++++++++---
 drivers/misc/mei/hw-me.h  |  21 +++++----
 drivers/misc/mei/pci-me.c | 107 ++++++++++++++++++++++++----------------------
 3 files changed, 94 insertions(+), 66 deletions(-)

diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index 71216affcab1..171d471fe66f 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -1376,38 +1376,58 @@ static bool mei_me_fw_type_sps(struct pci_dev *pdev)
 	.fw_status.status[5] = PCI_CFG_HFS_6
 
 /* ICH Legacy devices */
-const struct mei_cfg mei_me_legacy_cfg = {
+static const struct mei_cfg mei_me_legacy_cfg = {
 	MEI_CFG_LEGACY_HFS,
 };
 
 /* ICH devices */
-const struct mei_cfg mei_me_ich_cfg = {
+static const struct mei_cfg mei_me_ich_cfg = {
 	MEI_CFG_ICH_HFS,
 };
 
 /* PCH devices */
-const struct mei_cfg mei_me_pch_cfg = {
+static const struct mei_cfg mei_me_pch_cfg = {
 	MEI_CFG_PCH_HFS,
 };
 
 
 /* PCH Cougar Point and Patsburg with quirk for Node Manager exclusion */
-const struct mei_cfg mei_me_pch_cpt_pbg_cfg = {
+static const struct mei_cfg mei_me_pch_cpt_pbg_cfg = {
 	MEI_CFG_PCH_HFS,
 	MEI_CFG_FW_NM,
 };
 
 /* PCH8 Lynx Point and newer devices */
-const struct mei_cfg mei_me_pch8_cfg = {
+static const struct mei_cfg mei_me_pch8_cfg = {
 	MEI_CFG_PCH8_HFS,
 };
 
 /* PCH8 Lynx Point with quirk for SPS Firmware exclusion */
-const struct mei_cfg mei_me_pch8_sps_cfg = {
+static const struct mei_cfg mei_me_pch8_sps_cfg = {
 	MEI_CFG_PCH8_HFS,
 	MEI_CFG_FW_SPS,
 };
 
+static const struct mei_cfg *const mei_cfg_list[] = {
+	NULL,
+	&mei_me_legacy_cfg,
+	&mei_me_ich_cfg,
+	&mei_me_pch_cfg,
+	&mei_me_pch_cpt_pbg_cfg,
+	&mei_me_pch8_cfg,
+	&mei_me_pch8_sps_cfg,
+};
+
+const struct mei_cfg *mei_me_get_cfg(kernel_ulong_t idx)
+{
+	BUILD_BUG_ON(ARRAY_SIZE(mei_cfg_list) != MEI_ME_NUM_CFG);
+
+	if (idx >= MEI_ME_NUM_CFG)
+		return NULL;
+
+	return mei_cfg_list[idx];
+};
+
 /**
  * mei_me_dev_init - allocates and initializes the mei device structure
  *
diff --git a/drivers/misc/mei/hw-me.h b/drivers/misc/mei/hw-me.h
index cf64847a35b9..5fda83619044 100644
--- a/drivers/misc/mei/hw-me.h
+++ b/drivers/misc/mei/hw-me.h
@@ -41,8 +41,7 @@ struct mei_cfg {
 #define MEI_PCI_DEVICE(dev, cfg) \
 	.vendor = PCI_VENDOR_ID_INTEL, .device = (dev), \
 	.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID, \
-	.driver_data = (kernel_ulong_t)&(cfg)
-
+	.driver_data = (kernel_ulong_t)(cfg),
 
 #define MEI_ME_RPM_TIMEOUT    500 /* ms */
 
@@ -63,12 +62,18 @@ struct mei_me_hw {
 
 #define to_me_hw(dev) (struct mei_me_hw *)((dev)->hw)
 
-extern const struct mei_cfg mei_me_legacy_cfg;
-extern const struct mei_cfg mei_me_ich_cfg;
-extern const struct mei_cfg mei_me_pch_cfg;
-extern const struct mei_cfg mei_me_pch_cpt_pbg_cfg;
-extern const struct mei_cfg mei_me_pch8_cfg;
-extern const struct mei_cfg mei_me_pch8_sps_cfg;
+enum mei_cfg_idx {
+	MEI_ME_UNDEF_CFG,
+	MEI_ME_LEGACY_CFG,
+	MEI_ME_ICH_CFG,
+	MEI_ME_PCH_CFG,
+	MEI_ME_PCH_CPT_PBG_CFG,
+	MEI_ME_PCH8_CFG,
+	MEI_ME_PCH8_SPS_CFG,
+	MEI_ME_NUM_CFG,
+};
+
+const struct mei_cfg *mei_me_get_cfg(kernel_ulong_t idx);
 
 struct mei_device *mei_me_dev_init(struct pci_dev *pdev,
 				   const struct mei_cfg *cfg);
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index 8621a198a2ce..3c1c6aedf41b 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -43,57 +43,57 @@
 
 /* mei_pci_tbl - PCI Device ID Table */
 static const struct pci_device_id mei_me_pci_tbl[] = {
-	{MEI_PCI_DEVICE(MEI_DEV_ID_82946GZ, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_82G35, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_82Q965, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_82G965, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_82GM965, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_82GME965, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_82Q35, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_82G33, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_82Q33, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_82X38, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_3200, mei_me_legacy_cfg)},
-
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_6, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_7, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_8, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_9, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_10, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9M_1, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9M_2, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9M_3, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9M_4, mei_me_legacy_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH10_1, mei_me_ich_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH10_2, mei_me_ich_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH10_3, mei_me_ich_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH10_4, mei_me_ich_cfg)},
-
-	{MEI_PCI_DEVICE(MEI_DEV_ID_IBXPK_1, mei_me_pch_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_IBXPK_2, mei_me_pch_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_CPT_1, mei_me_pch_cpt_pbg_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_PBG_1, mei_me_pch_cpt_pbg_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_PPT_1, mei_me_pch_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_PPT_2, mei_me_pch_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_PPT_3, mei_me_pch_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_LPT_H, mei_me_pch8_sps_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_LPT_W, mei_me_pch8_sps_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_LPT_LP, mei_me_pch8_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_LPT_HR, mei_me_pch8_sps_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_WPT_LP, mei_me_pch8_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_WPT_LP_2, mei_me_pch8_cfg)},
-
-	{MEI_PCI_DEVICE(MEI_DEV_ID_SPT, mei_me_pch8_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_SPT_2, mei_me_pch8_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_SPT_H, mei_me_pch8_sps_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_SPT_H_2, mei_me_pch8_sps_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_LBG, mei_me_pch8_cfg)},
-
-	{MEI_PCI_DEVICE(MEI_DEV_ID_BXT_M, mei_me_pch8_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_APL_I, mei_me_pch8_cfg)},
-
-	{MEI_PCI_DEVICE(MEI_DEV_ID_KBP, mei_me_pch8_cfg)},
-	{MEI_PCI_DEVICE(MEI_DEV_ID_KBP_2, mei_me_pch8_cfg)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_82946GZ, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_82G35, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_82Q965, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_82G965, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_82GM965, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_82GME965, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_82Q35, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_82G33, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_82Q33, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_82X38, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_3200, MEI_ME_LEGACY_CFG)},
+
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_6, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_7, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_8, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_9, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9_10, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9M_1, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9M_2, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9M_3, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH9M_4, MEI_ME_LEGACY_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH10_1, MEI_ME_ICH_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH10_2, MEI_ME_ICH_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH10_3, MEI_ME_ICH_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_ICH10_4, MEI_ME_ICH_CFG)},
+
+	{MEI_PCI_DEVICE(MEI_DEV_ID_IBXPK_1, MEI_ME_PCH_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_IBXPK_2, MEI_ME_PCH_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_CPT_1, MEI_ME_PCH_CPT_PBG_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_PBG_1, MEI_ME_PCH_CPT_PBG_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_PPT_1, MEI_ME_PCH_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_PPT_2, MEI_ME_PCH_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_PPT_3, MEI_ME_PCH_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_LPT_H, MEI_ME_PCH8_SPS_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_LPT_W, MEI_ME_PCH8_SPS_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_LPT_LP, MEI_ME_PCH8_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_LPT_HR, MEI_ME_PCH8_SPS_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_WPT_LP, MEI_ME_PCH8_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_WPT_LP_2, MEI_ME_PCH8_CFG)},
+
+	{MEI_PCI_DEVICE(MEI_DEV_ID_SPT, MEI_ME_PCH8_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_SPT_2, MEI_ME_PCH8_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_SPT_H, MEI_ME_PCH8_SPS_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_SPT_H_2, MEI_ME_PCH8_SPS_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_LBG, MEI_ME_PCH8_CFG)},
+
+	{MEI_PCI_DEVICE(MEI_DEV_ID_BXT_M, MEI_ME_PCH8_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_APL_I, MEI_ME_PCH8_CFG)},
+
+	{MEI_PCI_DEVICE(MEI_DEV_ID_KBP, MEI_ME_PCH8_CFG)},
+	{MEI_PCI_DEVICE(MEI_DEV_ID_KBP_2, MEI_ME_PCH8_CFG)},
 
 	/* required last entry */
 	{0, }
@@ -138,12 +138,15 @@ static bool mei_me_quirk_probe(struct pci_dev *pdev,
  */
 static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
-	const struct mei_cfg *cfg = (struct mei_cfg *)(ent->driver_data);
+	const struct mei_cfg *cfg;
 	struct mei_device *dev;
 	struct mei_me_hw *hw;
 	unsigned int irqflags;
 	int err;
 
+	cfg = mei_me_get_cfg(ent->driver_data);
+	if (!cfg)
+		return -ENODEV;
 
 	if (!mei_me_quirk_probe(pdev, cfg))
 		return -ENODEV;
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [char-misc-next 3/3] mei: me: use an index instead of a pointer for private data
  2017-06-12  9:15 ` [char-misc-next 3/3] mei: me: use an index instead of a pointer for private data Tomas Winkler
@ 2017-06-13 13:46   ` Greg Kroah-Hartman
  2017-06-13 14:33     ` Winkler, Tomas
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2017-06-13 13:46 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: Alexander Usyskin, linux-kernel

On Mon, Jun 12, 2017 at 12:15:57PM +0300, Tomas Winkler wrote:
> +static const struct mei_cfg *const mei_cfg_list[] = {
> +	NULL,
> +	&mei_me_legacy_cfg,
> +	&mei_me_ich_cfg,
> +	&mei_me_pch_cfg,
> +	&mei_me_pch_cpt_pbg_cfg,
> +	&mei_me_pch8_cfg,
> +	&mei_me_pch8_sps_cfg,
> +};

Does this structure have to keep in sync with:

> +enum mei_cfg_idx {
> +	MEI_ME_UNDEF_CFG,
> +	MEI_ME_LEGACY_CFG,
> +	MEI_ME_ICH_CFG,
> +	MEI_ME_PCH_CFG,
> +	MEI_ME_PCH_CPT_PBG_CFG,
> +	MEI_ME_PCH8_CFG,
> +	MEI_ME_PCH8_SPS_CFG,
> +	MEI_ME_NUM_CFG,
> +};

That value?

If so, why not make it automatic and have the array use the enum values?
That way you know you get it right.

At the very least, document the heck out of this...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [char-misc-next 3/3] mei: me: use an index instead of a pointer for private data
  2017-06-13 13:46   ` Greg Kroah-Hartman
@ 2017-06-13 14:33     ` Winkler, Tomas
  0 siblings, 0 replies; 5+ messages in thread
From: Winkler, Tomas @ 2017-06-13 14:33 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Usyskin, Alexander, linux-kernel



> -----Original Message-----
> From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, June 13, 2017 16:46
> To: Winkler, Tomas <tomas.winkler@intel.com>
> Cc: Usyskin, Alexander <alexander.usyskin@intel.com>; linux-
> kernel@vger.kernel.org
> Subject: Re: [char-misc-next 3/3] mei: me: use an index instead of a pointer
> for private data
> 
> On Mon, Jun 12, 2017 at 12:15:57PM +0300, Tomas Winkler wrote:
> > +static const struct mei_cfg *const mei_cfg_list[] = {
> > +	NULL,
> > +	&mei_me_legacy_cfg,
> > +	&mei_me_ich_cfg,
> > +	&mei_me_pch_cfg,
> > +	&mei_me_pch_cpt_pbg_cfg,
> > +	&mei_me_pch8_cfg,
> > +	&mei_me_pch8_sps_cfg,
> > +};
> 
> Does this structure have to keep in sync with:
> 
> > +enum mei_cfg_idx {
> > +	MEI_ME_UNDEF_CFG,
> > +	MEI_ME_LEGACY_CFG,
> > +	MEI_ME_ICH_CFG,
> > +	MEI_ME_PCH_CFG,
> > +	MEI_ME_PCH_CPT_PBG_CFG,
> > +	MEI_ME_PCH8_CFG,
> > +	MEI_ME_PCH8_SPS_CFG,
> > +	MEI_ME_NUM_CFG,
> > +};
> 
> That value?
> 
> If so, why not make it automatic and have the array use the enum values?
> That way you know you get it right.

Do you mean 'designated Initializers'?  I didn't know it's supported.. neat feature. 

> At the very least, document the heck out of this...

Will do

Thanks
Tomas

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-06-13 14:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-12  9:15 [char-misc-next 1/3] mei: validate the message header only in first fragment Tomas Winkler
2017-06-12  9:15 ` [char-misc-next 2/3] mei: drop unreachable code in mei_start Tomas Winkler
2017-06-12  9:15 ` [char-misc-next 3/3] mei: me: use an index instead of a pointer for private data Tomas Winkler
2017-06-13 13:46   ` Greg Kroah-Hartman
2017-06-13 14:33     ` Winkler, Tomas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).