This patchset:
- Adds define for the channels number for mux device.
- Adds differed bus functionality.
- Changes input for device create routine in mlxreg-hotplug driver.
- Adds physical bus number auto detection.
Vadim Pasternak (4):
platform/x86: mlx-platform: Use define for the channel numbers
platform/x86: mlx-platform: Add differed bus functionality
platform/mellanox: mlxreg-hotplug: Change input for device create
routine
platform/x86: mlx-platform: Add physical bus number auto detection
drivers/platform/mellanox/mlxreg-hotplug.c | 31 +++++++++-----
drivers/platform/x86/mlx-platform.c | 68 ++++++++++++++++++++++++++++--
include/linux/platform_data/mlxreg.h | 4 ++
3 files changed, 90 insertions(+), 13 deletions(-)
--
2.1.4
Add physical bus number auto detection mechanism in order to avoid a
possible collision with I2C physical bus allocation. The mlx-platform
driver activates i2c-mlxcpld driver with no bus number specification. It
based on an assumption that on all Mellanox systems physical bus number is
supposed to be number one. On some X86 systems other I2C drivers, which
even are not used for Mellanox systems, could come up before i2c-mlxcpld,
for example i2c_i801, i2c_ismt, i2c-kempld. And in such case a pre-defined
I2C bus number one, could be busy. In order to avoid such situation,
with no having special kernel config file disabling unwanted modules or
with no putting anything to a blacklist, the logic for I2C available bus
number auto detection is added. To ensure it, mlx-platform driver verifies
which adapter number is free prior activation of i2c-mlxcpld driver. In
case numbered adapter one is busy, it shifts to the available number. This
shift is passed to i2c-mlxcpld driver, the mux base numbers are
incremented by the this shift value and passed to i2c-mux-reg driver, and
also this value is passed to mlxreg-hotplug driver.
Signed-off-by: Vadim Pasternak <[email protected]>
---
drivers/platform/mellanox/mlxreg-hotplug.c | 12 ++++---
drivers/platform/x86/mlx-platform.c | 53 ++++++++++++++++++++++++++++--
include/linux/platform_data/mlxreg.h | 2 ++
3 files changed, 61 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/mellanox/mlxreg-hotplug.c b/drivers/platform/mellanox/mlxreg-hotplug.c
index c1e1c4f..ea9e7f4 100644
--- a/drivers/platform/mellanox/mlxreg-hotplug.c
+++ b/drivers/platform/mellanox/mlxreg-hotplug.c
@@ -96,6 +96,8 @@ struct mlxreg_hotplug_priv_data {
static int mlxreg_hotplug_device_create(struct mlxreg_hotplug_priv_data *priv,
struct mlxreg_core_data *data)
{
+ struct mlxreg_core_hotplug_platform_data *pdata;
+
/*
* Return if adapter number is negative. It could be in case hotplug
* event is not associated with hotplug device.
@@ -103,10 +105,12 @@ static int mlxreg_hotplug_device_create(struct mlxreg_hotplug_priv_data *priv,
if (data->hpdev.nr < 0)
return 0;
- data->hpdev.adapter = i2c_get_adapter(data->hpdev.nr);
+ pdata = dev_get_platdata(&priv->pdev->dev);
+ data->hpdev.adapter = i2c_get_adapter(data->hpdev.nr +
+ pdata->shift_nr);
if (!data->hpdev.adapter) {
dev_err(priv->dev, "Failed to get adapter for bus %d\n",
- data->hpdev.nr);
+ data->hpdev.nr + pdata->shift_nr);
return -EFAULT;
}
@@ -114,8 +118,8 @@ static int mlxreg_hotplug_device_create(struct mlxreg_hotplug_priv_data *priv,
data->hpdev.brdinfo);
if (!data->hpdev.client) {
dev_err(priv->dev, "Failed to create client %s at bus %d at addr 0x%02x\n",
- data->hpdev.brdinfo->type, data->hpdev.nr,
- data->hpdev.brdinfo->addr);
+ data->hpdev.brdinfo->type, data->hpdev.nr +
+ pdata->shift_nr, data->hpdev.brdinfo->addr);
i2c_put_adapter(data->hpdev.adapter);
data->hpdev.adapter = NULL;
diff --git a/drivers/platform/x86/mlx-platform.c b/drivers/platform/x86/mlx-platform.c
index 67f3c6d..7a0bd24 100644
--- a/drivers/platform/x86/mlx-platform.c
+++ b/drivers/platform/x86/mlx-platform.c
@@ -85,6 +85,12 @@
#define MLXPLAT_CPLD_FAN_MASK GENMASK(3, 0)
#define MLXPLAT_CPLD_FAN_NG_MASK GENMASK(5, 0)
+/* Default I2C parent bus number */
+#define MLXPLAT_CPLD_PHYS_ADAPTER_DEF_NR 1
+
+/* Maximum number of possible physical buses equipped on system */
+#define MLXPLAT_CPLD_MAX_PHYS_ADAPTER_NUM 16
+
/* Number of channels in group */
#define MLXPLAT_CPLD_GRP_CHNL_NUM 8
@@ -843,10 +849,48 @@ static const struct dmi_system_id mlxplat_dmi_table[] __initconst = {
MODULE_DEVICE_TABLE(dmi, mlxplat_dmi_table);
+static int mlxplat_mlxcpld_verify_bus_topology(int *nr)
+{
+ struct i2c_adapter *search_adap;
+ int shift, i;
+
+ /* Scan adapters from expected id to verify it is free. */
+ *nr = MLXPLAT_CPLD_PHYS_ADAPTER_DEF_NR;
+ for (i = MLXPLAT_CPLD_PHYS_ADAPTER_DEF_NR; i <
+ MLXPLAT_CPLD_MAX_PHYS_ADAPTER_NUM; i++) {
+ search_adap = i2c_get_adapter(i);
+ if (search_adap) {
+ i2c_put_adapter(search_adap);
+ continue;
+ }
+
+ /* Return if expected parent adapter is free. */
+ if (i == MLXPLAT_CPLD_PHYS_ADAPTER_DEF_NR)
+ return 0;
+ break;
+ }
+
+ /* Return with error if free id for adapter is not found. */
+ if (i == MLXPLAT_CPLD_MAX_PHYS_ADAPTER_NUM)
+ return -ENODEV;
+
+ /* Shift adapter ids, since expected parent adapter is not free. */
+ *nr = i;
+ for (i = 0; i < ARRAY_SIZE(mlxplat_mux_data); i++) {
+ shift = *nr - mlxplat_mux_data[i].parent;
+ mlxplat_mux_data[i].parent = *nr;
+ mlxplat_mux_data[i].base_nr += shift;
+ if (shift > 0)
+ mlxplat_hotplug->shift_nr = shift;
+ }
+
+ return 0;
+}
+
static int __init mlxplat_init(void)
{
struct mlxplat_priv *priv;
- int i, err;
+ int i, nr, err;
if (!dmi_check_system(mlxplat_dmi_table))
return -ENODEV;
@@ -866,7 +910,12 @@ static int __init mlxplat_init(void)
}
platform_set_drvdata(mlxplat_dev, priv);
- priv->pdev_i2c = platform_device_register_simple("i2c_mlxcpld", -1,
+ err = mlxplat_mlxcpld_verify_bus_topology(&nr);
+ if (nr < 0)
+ goto fail_alloc;
+
+ nr = (nr == MLXPLAT_CPLD_MAX_PHYS_ADAPTER_NUM) ? -1 : nr;
+ priv->pdev_i2c = platform_device_register_simple("i2c_mlxcpld", nr,
NULL, 0);
if (IS_ERR(priv->pdev_i2c)) {
err = PTR_ERR(priv->pdev_i2c);
diff --git a/include/linux/platform_data/mlxreg.h b/include/linux/platform_data/mlxreg.h
index 2629109..2744cff 100644
--- a/include/linux/platform_data/mlxreg.h
+++ b/include/linux/platform_data/mlxreg.h
@@ -130,6 +130,7 @@ struct mlxreg_core_platform_data {
* @cell_low: location of low aggregation interrupt register;
* @mask_low: low aggregation interrupt common mask;
* @deferred_nr: I2C adapter number must be exist prior probing execution;
+ * @shift_nr: I2C adapter numbers must be incremented by this value;
*/
struct mlxreg_core_hotplug_platform_data {
struct mlxreg_core_item *items;
@@ -141,6 +142,7 @@ struct mlxreg_core_hotplug_platform_data {
u32 cell_low;
u32 mask_low;
int deferred_nr;
+ int shift_nr;
};
#endif /* __LINUX_PLATFORM_DATA_MLXREG_H */
--
2.1.4
Add deferred bus functionality in order to enforce probing flow execution
order. Driver mlx-platform activates platform driver i2c-mux-reg, which
creates busses infrastructure, after that it activates mlxreg-hotplug
driver, which uses these busses, for connecting devices. The possible
miss-ordering can happened, for example in case the probing routine of
mlxreg-hotplug is already started execution, while i2c-mux-reg probing
routine is not completed yet. In such situation the first one could
attempt to connect device to adapter number, which is not created yet.
And as a result this connection will fail. In order to ensure the order of
probing execution on mlxreg-hotplug probe routine will be deferred until
all the busses is not created by probe routine of i2c-mux-reg.
In order to ensure the flow order, mlx-platform driver passes the highest
bus number to the mlxreg-hotplug platform data, which in their turn could
wait in the deferred state, until all the necessary buses topology is not
exist.
Signed-off-by: Vadim Pasternak <[email protected]>
---
drivers/platform/mellanox/mlxreg-hotplug.c | 7 +++++++
drivers/platform/x86/mlx-platform.c | 10 ++++++++++
include/linux/platform_data/mlxreg.h | 2 ++
3 files changed, 19 insertions(+)
diff --git a/drivers/platform/mellanox/mlxreg-hotplug.c b/drivers/platform/mellanox/mlxreg-hotplug.c
index 313cf8a..fe4910b 100644
--- a/drivers/platform/mellanox/mlxreg-hotplug.c
+++ b/drivers/platform/mellanox/mlxreg-hotplug.c
@@ -550,6 +550,7 @@ static int mlxreg_hotplug_probe(struct platform_device *pdev)
{
struct mlxreg_core_hotplug_platform_data *pdata;
struct mlxreg_hotplug_priv_data *priv;
+ struct i2c_adapter *deferred_adap;
int err;
pdata = dev_get_platdata(&pdev->dev);
@@ -558,6 +559,12 @@ static int mlxreg_hotplug_probe(struct platform_device *pdev)
return -EINVAL;
}
+ /* Defer probing if the necessary adapter is not configured yet. */
+ deferred_adap = i2c_get_adapter(pdata->deferred_nr);
+ if (!deferred_adap)
+ return -EPROBE_DEFER;
+ i2c_put_adapter(deferred_adap);
+
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
diff --git a/drivers/platform/x86/mlx-platform.c b/drivers/platform/x86/mlx-platform.c
index 71b452a..67f3c6d 100644
--- a/drivers/platform/x86/mlx-platform.c
+++ b/drivers/platform/x86/mlx-platform.c
@@ -697,6 +697,8 @@ static int __init mlxplat_dmi_default_matched(const struct dmi_system_id *dmi)
ARRAY_SIZE(mlxplat_default_channels[i]);
}
mlxplat_hotplug = &mlxplat_mlxcpld_default_data;
+ mlxplat_hotplug->deferred_nr =
+ mlxplat_default_channels[i - 1][MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
return 1;
};
@@ -711,6 +713,8 @@ static int __init mlxplat_dmi_msn21xx_matched(const struct dmi_system_id *dmi)
ARRAY_SIZE(mlxplat_msn21xx_channels);
}
mlxplat_hotplug = &mlxplat_mlxcpld_msn21xx_data;
+ mlxplat_hotplug->deferred_nr =
+ mlxplat_msn21xx_channels[MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
return 1;
};
@@ -725,6 +729,8 @@ static int __init mlxplat_dmi_msn274x_matched(const struct dmi_system_id *dmi)
ARRAY_SIZE(mlxplat_msn21xx_channels);
}
mlxplat_hotplug = &mlxplat_mlxcpld_msn274x_data;
+ mlxplat_hotplug->deferred_nr =
+ mlxplat_msn21xx_channels[MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
return 1;
};
@@ -739,6 +745,8 @@ static int __init mlxplat_dmi_msn201x_matched(const struct dmi_system_id *dmi)
ARRAY_SIZE(mlxplat_msn21xx_channels);
}
mlxplat_hotplug = &mlxplat_mlxcpld_msn201x_data;
+ mlxplat_hotplug->deferred_nr =
+ mlxplat_default_channels[i - 1][MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
return 1;
};
@@ -753,6 +761,8 @@ static int __init mlxplat_dmi_qmb7xx_matched(const struct dmi_system_id *dmi)
ARRAY_SIZE(mlxplat_msn21xx_channels);
}
mlxplat_hotplug = &mlxplat_mlxcpld_default_ng_data;
+ mlxplat_hotplug->deferred_nr =
+ mlxplat_msn21xx_channels[MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
return 1;
};
diff --git a/include/linux/platform_data/mlxreg.h b/include/linux/platform_data/mlxreg.h
index fcdc707..2629109 100644
--- a/include/linux/platform_data/mlxreg.h
+++ b/include/linux/platform_data/mlxreg.h
@@ -129,6 +129,7 @@ struct mlxreg_core_platform_data {
* @mask: top aggregation interrupt common mask;
* @cell_low: location of low aggregation interrupt register;
* @mask_low: low aggregation interrupt common mask;
+ * @deferred_nr: I2C adapter number must be exist prior probing execution;
*/
struct mlxreg_core_hotplug_platform_data {
struct mlxreg_core_item *items;
@@ -139,6 +140,7 @@ struct mlxreg_core_hotplug_platform_data {
u32 mask;
u32 cell_low;
u32 mask_low;
+ int deferred_nr;
};
#endif /* __LINUX_PLATFORM_DATA_MLXREG_H */
--
2.1.4
Change the first input parameter in mlxreg_hotplug_device_create to the
pointer to mlxreg_hotplug private data in order to allow to use the fields
from the private data structure inside this routine.
Signed-off-by: Vadim Pasternak <[email protected]>
---
drivers/platform/mellanox/mlxreg-hotplug.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/mellanox/mlxreg-hotplug.c b/drivers/platform/mellanox/mlxreg-hotplug.c
index fe4910b..c1e1c4f 100644
--- a/drivers/platform/mellanox/mlxreg-hotplug.c
+++ b/drivers/platform/mellanox/mlxreg-hotplug.c
@@ -93,7 +93,7 @@ struct mlxreg_hotplug_priv_data {
bool after_probe;
};
-static int mlxreg_hotplug_device_create(struct device *dev,
+static int mlxreg_hotplug_device_create(struct mlxreg_hotplug_priv_data *priv,
struct mlxreg_core_data *data)
{
/*
@@ -105,7 +105,7 @@ static int mlxreg_hotplug_device_create(struct device *dev,
data->hpdev.adapter = i2c_get_adapter(data->hpdev.nr);
if (!data->hpdev.adapter) {
- dev_err(dev, "Failed to get adapter for bus %d\n",
+ dev_err(priv->dev, "Failed to get adapter for bus %d\n",
data->hpdev.nr);
return -EFAULT;
}
@@ -113,7 +113,7 @@ static int mlxreg_hotplug_device_create(struct device *dev,
data->hpdev.client = i2c_new_device(data->hpdev.adapter,
data->hpdev.brdinfo);
if (!data->hpdev.client) {
- dev_err(dev, "Failed to create client %s at bus %d at addr 0x%02x\n",
+ dev_err(priv->dev, "Failed to create client %s at bus %d at addr 0x%02x\n",
data->hpdev.brdinfo->type, data->hpdev.nr,
data->hpdev.brdinfo->addr);
@@ -270,10 +270,10 @@ mlxreg_hotplug_work_helper(struct mlxreg_hotplug_priv_data *priv,
if (item->inversed)
mlxreg_hotplug_device_destroy(data);
else
- mlxreg_hotplug_device_create(priv->dev, data);
+ mlxreg_hotplug_device_create(priv, data);
} else {
if (item->inversed)
- mlxreg_hotplug_device_create(priv->dev, data);
+ mlxreg_hotplug_device_create(priv, data);
else
mlxreg_hotplug_device_destroy(data);
}
@@ -319,7 +319,7 @@ mlxreg_hotplug_health_work_helper(struct mlxreg_hotplug_priv_data *priv,
if (regval == MLXREG_HOTPLUG_HEALTH_MASK) {
if ((data->health_cntr++ == MLXREG_HOTPLUG_RST_CNTR) ||
!priv->after_probe) {
- mlxreg_hotplug_device_create(priv->dev, data);
+ mlxreg_hotplug_device_create(priv, data);
data->attached = true;
}
} else {
--
2.1.4
Add define for the channels number for mux device, instead of using
hardcoded value inside the code in order to improve code readability.
Signed-off-by: Vadim Pasternak <[email protected]>
---
drivers/platform/x86/mlx-platform.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/mlx-platform.c b/drivers/platform/x86/mlx-platform.c
index 454e14f..71b452a 100644
--- a/drivers/platform/x86/mlx-platform.c
+++ b/drivers/platform/x86/mlx-platform.c
@@ -85,6 +85,9 @@
#define MLXPLAT_CPLD_FAN_MASK GENMASK(3, 0)
#define MLXPLAT_CPLD_FAN_NG_MASK GENMASK(5, 0)
+/* Number of channels in group */
+#define MLXPLAT_CPLD_GRP_CHNL_NUM 8
+
/* Start channel numbers */
#define MLXPLAT_CPLD_CH1 2
#define MLXPLAT_CPLD_CH2 10
@@ -124,7 +127,7 @@ static const struct resource mlxplat_lpc_resources[] = {
};
/* Platform default channels */
-static const int mlxplat_default_channels[][8] = {
+static const int mlxplat_default_channels[][MLXPLAT_CPLD_GRP_CHNL_NUM] = {
{
MLXPLAT_CPLD_CH1, MLXPLAT_CPLD_CH1 + 1, MLXPLAT_CPLD_CH1 + 2,
MLXPLAT_CPLD_CH1 + 3, MLXPLAT_CPLD_CH1 + 4, MLXPLAT_CPLD_CH1 +
--
2.1.4
On Tue, Feb 13, 2018 at 10:09:32PM +0000, Vadim Pasternak wrote:
> This patchset:
> - Adds define for the channels number for mux device.
> - Adds differed bus functionality.
> - Changes input for device create routine in mlxreg-hotplug driver.
> - Adds physical bus number auto detection.
Hi Vadim,
We are now in the RC cycle for 4.16. Do you consider these changes to be
"fixes" that need to be included in 4.16? It was difficult to tell from
the commit messages how severe some of the possible issues were.
I'm concerned about the level of testing seen by the previous patch
series if we are getting these changes so soon after they were merged.
Can you comment on why these are being discovered now - and how
confident are we in the drivers with these changes applied?
>
> Vadim Pasternak (4):
> platform/x86: mlx-platform: Use define for the channel numbers
> platform/x86: mlx-platform: Add differed bus functionality
> platform/mellanox: mlxreg-hotplug: Change input for device create
> routine
> platform/x86: mlx-platform: Add physical bus number auto detection
>
> drivers/platform/mellanox/mlxreg-hotplug.c | 31 +++++++++-----
> drivers/platform/x86/mlx-platform.c | 68 ++++++++++++++++++++++++++++--
> include/linux/platform_data/mlxreg.h | 4 ++
> 3 files changed, 90 insertions(+), 13 deletions(-)
>
> --
> 2.1.4
>
>
--
Darren Hart
VMware Open Source Technology Center
> -----Original Message-----
> From: Darren Hart [mailto:[email protected]]
> Sent: Friday, February 16, 2018 3:33 AM
> To: Vadim Pasternak <[email protected]>
> Cc: [email protected]; [email protected]; platform-
> [email protected]; [email protected]; [email protected];
> Michael Shych <[email protected]>
> Subject: Re: [PATCH v1 0/4] platform/x86: mlx-platform: Add bus differed and
> auto detection functionalities
>
> On Tue, Feb 13, 2018 at 10:09:32PM +0000, Vadim Pasternak wrote:
> > This patchset:
> > - Adds define for the channels number for mux device.
> > - Adds differed bus functionality.
> > - Changes input for device create routine in mlxreg-hotplug driver.
> > - Adds physical bus number auto detection.
>
> Hi Vadim,
Hi Darren,
>
> We are now in the RC cycle for 4.16. Do you consider these changes to be
> "fixes" that need to be included in 4.16? It was difficult to tell from the commit
> messages how severe some of the possible issues were.
I actually considered these patches for the next, having in mind that 4.16 is
already closed.
>
> I'm concerned about the level of testing seen by the previous patch series if we
> are getting these changes so soon after they were merged.
> Can you comment on why these are being discovered now - and how confident
> are we in the drivers with these changes applied?
These patches has been tested on the all relevant platforms.
It has been discovered some time ago and it was in my plans to submit these
patches after the previous series.
Do you think it's better to consider this series as a bug fix, and target it to
4.16?
Thanks,
Vadim.
>
> >
> > Vadim Pasternak (4):
> > platform/x86: mlx-platform: Use define for the channel numbers
> > platform/x86: mlx-platform: Add differed bus functionality
> > platform/mellanox: mlxreg-hotplug: Change input for device create
> > routine
> > platform/x86: mlx-platform: Add physical bus number auto detection
> >
> > drivers/platform/mellanox/mlxreg-hotplug.c | 31 +++++++++-----
> > drivers/platform/x86/mlx-platform.c | 68
> ++++++++++++++++++++++++++++--
> > include/linux/platform_data/mlxreg.h | 4 ++
> > 3 files changed, 90 insertions(+), 13 deletions(-)
> >
> > --
> > 2.1.4
> >
> >
>
> --
> Darren Hart
> VMware Open Source Technology Center
On Tue, Feb 13, 2018 at 10:09:34PM +0000, Vadim Pasternak wrote:
> Add deferred bus functionality in order to enforce probing flow execution
> order. Driver mlx-platform activates platform driver i2c-mux-reg, which
> creates busses infrastructure, after that it activates mlxreg-hotplug
> driver, which uses these busses, for connecting devices. The possible
> miss-ordering can happened, for example in case the probing routine of
> mlxreg-hotplug is already started execution, while i2c-mux-reg probing
> routine is not completed yet. In such situation the first one could
> attempt to connect device to adapter number, which is not created yet.
> And as a result this connection will fail. In order to ensure the order of
> probing execution on mlxreg-hotplug probe routine will be deferred until
> all the busses is not created by probe routine of i2c-mux-reg.
> In order to ensure the flow order, mlx-platform driver passes the highest
> bus number to the mlxreg-hotplug platform data, which in their turn could
> wait in the deferred state, until all the necessary buses topology is not
> exist.
Vadim,
I've got part way through a review of this series several times, and keep
running out of time trying to determine if the solution is appropriate
for the problem.
The deferred probing waiting for the i2c bus to be available seems like
something we might be able to handle more elegantly with:
1) request_module() in the platform driver
2) careful use of init levels (subsys, device, late)
This isn't something I'm well versed in - so I'll reach out here for
some advice from Greg and the i2c maintainer, Wolfram, and list (added
to Cc).
Thanks,
>
> Signed-off-by: Vadim Pasternak <[email protected]>
> ---
> drivers/platform/mellanox/mlxreg-hotplug.c | 7 +++++++
> drivers/platform/x86/mlx-platform.c | 10 ++++++++++
> include/linux/platform_data/mlxreg.h | 2 ++
> 3 files changed, 19 insertions(+)
>
> diff --git a/drivers/platform/mellanox/mlxreg-hotplug.c b/drivers/platform/mellanox/mlxreg-hotplug.c
> index 313cf8a..fe4910b 100644
> --- a/drivers/platform/mellanox/mlxreg-hotplug.c
> +++ b/drivers/platform/mellanox/mlxreg-hotplug.c
> @@ -550,6 +550,7 @@ static int mlxreg_hotplug_probe(struct platform_device *pdev)
> {
> struct mlxreg_core_hotplug_platform_data *pdata;
> struct mlxreg_hotplug_priv_data *priv;
> + struct i2c_adapter *deferred_adap;
> int err;
>
> pdata = dev_get_platdata(&pdev->dev);
> @@ -558,6 +559,12 @@ static int mlxreg_hotplug_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> + /* Defer probing if the necessary adapter is not configured yet. */
> + deferred_adap = i2c_get_adapter(pdata->deferred_nr);
> + if (!deferred_adap)
> + return -EPROBE_DEFER;
> + i2c_put_adapter(deferred_adap);
> +
> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> if (!priv)
> return -ENOMEM;
> diff --git a/drivers/platform/x86/mlx-platform.c b/drivers/platform/x86/mlx-platform.c
> index 71b452a..67f3c6d 100644
> --- a/drivers/platform/x86/mlx-platform.c
> +++ b/drivers/platform/x86/mlx-platform.c
> @@ -697,6 +697,8 @@ static int __init mlxplat_dmi_default_matched(const struct dmi_system_id *dmi)
> ARRAY_SIZE(mlxplat_default_channels[i]);
> }
> mlxplat_hotplug = &mlxplat_mlxcpld_default_data;
> + mlxplat_hotplug->deferred_nr =
> + mlxplat_default_channels[i - 1][MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
>
> return 1;
> };
> @@ -711,6 +713,8 @@ static int __init mlxplat_dmi_msn21xx_matched(const struct dmi_system_id *dmi)
> ARRAY_SIZE(mlxplat_msn21xx_channels);
> }
> mlxplat_hotplug = &mlxplat_mlxcpld_msn21xx_data;
> + mlxplat_hotplug->deferred_nr =
> + mlxplat_msn21xx_channels[MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
>
> return 1;
> };
> @@ -725,6 +729,8 @@ static int __init mlxplat_dmi_msn274x_matched(const struct dmi_system_id *dmi)
> ARRAY_SIZE(mlxplat_msn21xx_channels);
> }
> mlxplat_hotplug = &mlxplat_mlxcpld_msn274x_data;
> + mlxplat_hotplug->deferred_nr =
> + mlxplat_msn21xx_channels[MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
>
> return 1;
> };
> @@ -739,6 +745,8 @@ static int __init mlxplat_dmi_msn201x_matched(const struct dmi_system_id *dmi)
> ARRAY_SIZE(mlxplat_msn21xx_channels);
> }
> mlxplat_hotplug = &mlxplat_mlxcpld_msn201x_data;
> + mlxplat_hotplug->deferred_nr =
> + mlxplat_default_channels[i - 1][MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
>
> return 1;
> };
> @@ -753,6 +761,8 @@ static int __init mlxplat_dmi_qmb7xx_matched(const struct dmi_system_id *dmi)
> ARRAY_SIZE(mlxplat_msn21xx_channels);
> }
> mlxplat_hotplug = &mlxplat_mlxcpld_default_ng_data;
> + mlxplat_hotplug->deferred_nr =
> + mlxplat_msn21xx_channels[MLXPLAT_CPLD_GRP_CHNL_NUM - 1];
>
> return 1;
> };
> diff --git a/include/linux/platform_data/mlxreg.h b/include/linux/platform_data/mlxreg.h
> index fcdc707..2629109 100644
> --- a/include/linux/platform_data/mlxreg.h
> +++ b/include/linux/platform_data/mlxreg.h
> @@ -129,6 +129,7 @@ struct mlxreg_core_platform_data {
> * @mask: top aggregation interrupt common mask;
> * @cell_low: location of low aggregation interrupt register;
> * @mask_low: low aggregation interrupt common mask;
> + * @deferred_nr: I2C adapter number must be exist prior probing execution;
> */
> struct mlxreg_core_hotplug_platform_data {
> struct mlxreg_core_item *items;
> @@ -139,6 +140,7 @@ struct mlxreg_core_hotplug_platform_data {
> u32 mask;
> u32 cell_low;
> u32 mask_low;
> + int deferred_nr;
> };
>
> #endif /* __LINUX_PLATFORM_DATA_MLXREG_H */
> --
> 2.1.4
>
>
--
Darren Hart
VMware Open Source Technology Center
On Wed, Mar 21, 2018 at 03:58:18PM -0700, Darren Hart wrote:
> On Tue, Feb 13, 2018 at 10:09:34PM +0000, Vadim Pasternak wrote:
> > Add deferred bus functionality in order to enforce probing flow execution
> > order. Driver mlx-platform activates platform driver i2c-mux-reg, which
> > creates busses infrastructure, after that it activates mlxreg-hotplug
> > driver, which uses these busses, for connecting devices. The possible
> > miss-ordering can happened, for example in case the probing routine of
> > mlxreg-hotplug is already started execution, while i2c-mux-reg probing
> > routine is not completed yet. In such situation the first one could
> > attempt to connect device to adapter number, which is not created yet.
> > And as a result this connection will fail. In order to ensure the order of
> > probing execution on mlxreg-hotplug probe routine will be deferred until
> > all the busses is not created by probe routine of i2c-mux-reg.
> > In order to ensure the flow order, mlx-platform driver passes the highest
> > bus number to the mlxreg-hotplug platform data, which in their turn could
> > wait in the deferred state, until all the necessary buses topology is not
> > exist.
>
> Vadim,
>
> I've got part way through a review of this series several times, and keep
> running out of time trying to determine if the solution is appropriate
> for the problem.
>
> The deferred probing waiting for the i2c bus to be available seems like
> something we might be able to handle more elegantly with:
>
> 1) request_module() in the platform driver
> 2) careful use of init levels (subsys, device, late)
>
> This isn't something I'm well versed in - so I'll reach out here for
> some advice from Greg and the i2c maintainer, Wolfram, and list (added
> to Cc).
deferred device probe?
On Thu, Mar 22, 2018 at 10:04:51AM +0100, Greg Kroah-Hartman wrote:
> On Wed, Mar 21, 2018 at 03:58:18PM -0700, Darren Hart wrote:
> > On Tue, Feb 13, 2018 at 10:09:34PM +0000, Vadim Pasternak wrote:
> > > Add deferred bus functionality in order to enforce probing flow execution
> > > order. Driver mlx-platform activates platform driver i2c-mux-reg, which
> > > creates busses infrastructure, after that it activates mlxreg-hotplug
> > > driver, which uses these busses, for connecting devices. The possible
> > > miss-ordering can happened, for example in case the probing routine of
> > > mlxreg-hotplug is already started execution, while i2c-mux-reg probing
> > > routine is not completed yet. In such situation the first one could
> > > attempt to connect device to adapter number, which is not created yet.
> > > And as a result this connection will fail. In order to ensure the order of
> > > probing execution on mlxreg-hotplug probe routine will be deferred until
> > > all the busses is not created by probe routine of i2c-mux-reg.
> > > In order to ensure the flow order, mlx-platform driver passes the highest
> > > bus number to the mlxreg-hotplug platform data, which in their turn could
> > > wait in the deferred state, until all the necessary buses topology is not
> > > exist.
> >
> > Vadim,
> >
> > I've got part way through a review of this series several times, and keep
> > running out of time trying to determine if the solution is appropriate
> > for the problem.
> >
> > The deferred probing waiting for the i2c bus to be available seems like
> > something we might be able to handle more elegantly with:
> >
> > 1) request_module() in the platform driver
> > 2) careful use of init levels (subsys, device, late)
> >
> > This isn't something I'm well versed in - so I'll reach out here for
> > some advice from Greg and the i2c maintainer, Wolfram, and list (added
> > to Cc).
>
> deferred device probe?
>
This patch series adds the following, which I believe is what you're referring
to Greg? :
+ /* Defer probing if the necessary adapter is not configured yet. */
+ deferred_adap = i2c_get_adapter(pdata->deferred_nr);
+ if (!deferred_adap)
+ return -EPROBE_DEFER;
This is the preferred approach? Have the platform driver define the expected
adapter number, and then defer until that becomes available?
--
Darren Hart
VMware Open Source Technology Center
On Thu, Mar 22, 2018 at 03:18:40PM -0700, Darren Hart wrote:
> On Thu, Mar 22, 2018 at 10:04:51AM +0100, Greg Kroah-Hartman wrote:
> > On Wed, Mar 21, 2018 at 03:58:18PM -0700, Darren Hart wrote:
> > > On Tue, Feb 13, 2018 at 10:09:34PM +0000, Vadim Pasternak wrote:
> > > > Add deferred bus functionality in order to enforce probing flow execution
> > > > order. Driver mlx-platform activates platform driver i2c-mux-reg, which
> > > > creates busses infrastructure, after that it activates mlxreg-hotplug
> > > > driver, which uses these busses, for connecting devices. The possible
> > > > miss-ordering can happened, for example in case the probing routine of
> > > > mlxreg-hotplug is already started execution, while i2c-mux-reg probing
> > > > routine is not completed yet. In such situation the first one could
> > > > attempt to connect device to adapter number, which is not created yet.
> > > > And as a result this connection will fail. In order to ensure the order of
> > > > probing execution on mlxreg-hotplug probe routine will be deferred until
> > > > all the busses is not created by probe routine of i2c-mux-reg.
> > > > In order to ensure the flow order, mlx-platform driver passes the highest
> > > > bus number to the mlxreg-hotplug platform data, which in their turn could
> > > > wait in the deferred state, until all the necessary buses topology is not
> > > > exist.
> > >
> > > Vadim,
> > >
> > > I've got part way through a review of this series several times, and keep
> > > running out of time trying to determine if the solution is appropriate
> > > for the problem.
> > >
> > > The deferred probing waiting for the i2c bus to be available seems like
> > > something we might be able to handle more elegantly with:
> > >
> > > 1) request_module() in the platform driver
> > > 2) careful use of init levels (subsys, device, late)
> > >
> > > This isn't something I'm well versed in - so I'll reach out here for
> > > some advice from Greg and the i2c maintainer, Wolfram, and list (added
> > > to Cc).
> >
> > deferred device probe?
> >
>
> This patch series adds the following, which I believe is what you're referring
> to Greg? :
>
> + /* Defer probing if the necessary adapter is not configured yet. */
> + deferred_adap = i2c_get_adapter(pdata->deferred_nr);
> + if (!deferred_adap)
> + return -EPROBE_DEFER;
>
> This is the preferred approach? Have the platform driver define the expected
> adapter number, and then defer until that becomes available?
Yes, that is what that interface is for, so it should work like this.
But someone should test of course :)
greg k-h
On Tue, Feb 13, 2018 at 10:09:32PM +0000, Vadim Pasternak wrote:
> This patchset:
> - Adds define for the channels number for mux device.
> - Adds differed bus functionality.
> - Changes input for device create routine in mlxreg-hotplug driver.
> - Adds physical bus number auto detection.
>
> Vadim Pasternak (4):
> platform/x86: mlx-platform: Use define for the channel numbers
> platform/x86: mlx-platform: Add differed bus functionality
> platform/mellanox: mlxreg-hotplug: Change input for device create
> routine
> platform/x86: mlx-platform: Add physical bus number auto detection
Vadim,
I have rewritten most of the commit messages after parsing through them
and reviewing the code. I believe they are much more concise now, and
make the intent of the patches much clearer. I have not changed the
patch content (code).
Please review the commit messages for the top 4 commits:
http://git.infradead.org/linux-platform-drivers-x86.git/shortlog/refs/heads/review-dvhart-mellanox
Does this still match your intent?
--
Darren Hart
VMware Open Source Technology Center
> -----Original Message-----
> From: Darren Hart [mailto:[email protected]]
> Sent: Friday, March 23, 2018 1:56 AM
> To: Vadim Pasternak <[email protected]>
> Cc: [email protected]; [email protected]; platform-
> [email protected]; [email protected]; [email protected];
> Michael Shych <[email protected]>
> Subject: Re: [PATCH v1 0/4] platform/x86: mlx-platform: Add bus differed and
> auto detection functionalities
>
> On Tue, Feb 13, 2018 at 10:09:32PM +0000, Vadim Pasternak wrote:
> > This patchset:
> > - Adds define for the channels number for mux device.
> > - Adds differed bus functionality.
> > - Changes input for device create routine in mlxreg-hotplug driver.
> > - Adds physical bus number auto detection.
> >
> > Vadim Pasternak (4):
> > platform/x86: mlx-platform: Use define for the channel numbers
> > platform/x86: mlx-platform: Add differed bus functionality
> > platform/mellanox: mlxreg-hotplug: Change input for device create
> > routine
> > platform/x86: mlx-platform: Add physical bus number auto detection
>
> Vadim,
>
> I have rewritten most of the commit messages after parsing through them and
> reviewing the code. I believe they are much more concise now, and make the
> intent of the patches much clearer. I have not changed the patch content (code).
>
> Please review the commit messages for the top 4 commits:
> http://git.infradead.org/linux-platform-drivers-
> x86.git/shortlog/refs/heads/review-dvhart-mellanox
>
> Does this still match your intent?
>
Hi Darren,
Thank you very much for review.
Looks good.
Thanks,
Vadim.
> --
> Darren Hart
> VMware Open Source Technology Center
On Fri, Mar 23, 2018 at 03:59:22PM +0000, Vadim Pasternak wrote:
>
>
> > -----Original Message-----
> > From: Darren Hart [mailto:[email protected]]
> > Sent: Friday, March 23, 2018 1:56 AM
> > To: Vadim Pasternak <[email protected]>
> > Cc: [email protected]; [email protected]; platform-
> > [email protected]; [email protected]; [email protected];
> > Michael Shych <[email protected]>
> > Subject: Re: [PATCH v1 0/4] platform/x86: mlx-platform: Add bus differed and
> > auto detection functionalities
> >
> > On Tue, Feb 13, 2018 at 10:09:32PM +0000, Vadim Pasternak wrote:
> > > This patchset:
> > > - Adds define for the channels number for mux device.
> > > - Adds differed bus functionality.
> > > - Changes input for device create routine in mlxreg-hotplug driver.
> > > - Adds physical bus number auto detection.
> > >
> > > Vadim Pasternak (4):
> > > platform/x86: mlx-platform: Use define for the channel numbers
> > > platform/x86: mlx-platform: Add differed bus functionality
> > > platform/mellanox: mlxreg-hotplug: Change input for device create
> > > routine
> > > platform/x86: mlx-platform: Add physical bus number auto detection
> >
> > Vadim,
> >
> > I have rewritten most of the commit messages after parsing through them and
> > reviewing the code. I believe they are much more concise now, and make the
> > intent of the patches much clearer. I have not changed the patch content (code).
> >
> > Please review the commit messages for the top 4 commits:
> > http://git.infradead.org/linux-platform-drivers-
> > x86.git/shortlog/refs/heads/review-dvhart-mellanox
> >
> > Does this still match your intent?
> >
>
> Hi Darren,
> Thank you very much for review.
>
> Looks good.
Thanks Vadim. I have queued these plus the Kconfig build dep fix for 4.17.
--
Darren Hart
VMware Open Source Technology Center