| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b0035c61-7327-adfc-6ff7-60f0f275b1f4@axentia.se>
Date: Wed, 11 May 2022 17:43:34 +0200
From: Peter Rosin <peda@...ntia.se>
To: Kavyasree.Kotagiri@...rochip.com
Cc: devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, lee.jones@...aro.org,
linux@...linux.org.uk, Manohar.Puri@...rochip.com,
UNGLinuxDriver@...rochip.com, krzysztof.kozlowski+dt@...aro.org,
Nicolas.Ferre@...rochip.com, alexandre.belloni@...tlin.com,
Claudiu.Beznea@...rochip.com
Subject: Re: [PATCH v2 4/4] mux: lan966: Add support for flexcom mux
controller
Hi!
2022-05-11 at 16:28, Kavyasree.Kotagiri@...rochip.com wrote:
>> 2022-05-10 at 16:59, Kavyasree.Kotagiri@...rochip.com wrote:
>>>>> LAN966 SoC have 5 flexcoms. Each flexcom has 2 chip-selects.
>>>>> For each chip select of each flexcom there is a configuration
>>>>> register FLEXCOM_SHARED[0-4]:SS_MASK[0-1]. The width of
>>>>> configuration register is 21 because there are 21 shared pins
>>>>> on each of which the chip select can be mapped. Each bit of the
>>>>> register represents a different FLEXCOM_SHARED pin.
>>>>>
>>>>> Signed-off-by: Kavyasree Kotagiri <kavyasree.kotagiri@...rochip.com>
>>>>> ---
>>>>> arch/arm/mach-at91/Kconfig | 2 +
>>>>> drivers/mfd/atmel-flexcom.c | 55 +++++++++++++++-
>>>>> drivers/mux/Kconfig | 12 ++++
>>>>> drivers/mux/Makefile | 2 +
>>>>> drivers/mux/lan966-flx.c | 121
>>>> ++++++++++++++++++++++++++++++++++++
>>>>> 5 files changed, 191 insertions(+), 1 deletion(-)
>>>>> create mode 100644 drivers/mux/lan966-flx.c
>>>>>
>>>>> diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
>>>>> index 279810381256..26fb0f4e1b79 100644
>>>>> --- a/arch/arm/mach-at91/Kconfig
>>>>> +++ b/arch/arm/mach-at91/Kconfig
>>>>> @@ -74,6 +74,8 @@ config SOC_LAN966
>>>>> select DW_APB_TIMER_OF
>>>>> select ARM_GIC
>>>>> select MEMORY
>>>>> + select MULTIPLEXER
>>>>> + select MUX_LAN966
>>>>> help
>>>>> This enables support for ARMv7 based Microchip LAN966 SoC
>> family.
>>>>>
>>>>> diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
>>>>> index 559eb4d352b6..7cfd0fc3f4f0 100644
>>>>> --- a/drivers/mfd/atmel-flexcom.c
>>>>> +++ b/drivers/mfd/atmel-flexcom.c
>>>>> @@ -17,6 +17,7 @@
>>>>> #include <linux/io.h>
>>>>> #include <linux/clk.h>
>>>>> #include <dt-bindings/mfd/atmel-flexcom.h>
>>>>> +#include <linux/mux/consumer.h>
>>>>>
>>>>> /* I/O register offsets */
>>>>> #define FLEX_MR 0x0 /* Mode Register */
>>>>> @@ -28,6 +29,10 @@
>>>>> #define FLEX_MR_OPMODE(opmode) (((opmode) <<
>>>> FLEX_MR_OPMODE_OFFSET) & \
>>>>> FLEX_MR_OPMODE_MASK)
>>>>>
>>>>> +struct atmel_flex_caps {
>>>>> + bool has_flx_mux;
>>>>> +};
>>>>> +
>>>>> struct atmel_flexcom {
>>>>> void __iomem *base;
>>>>> u32 opmode;
>>>>> @@ -37,6 +42,7 @@ struct atmel_flexcom {
>>>>> static int atmel_flexcom_probe(struct platform_device *pdev)
>>>>> {
>>>>> struct device_node *np = pdev->dev.of_node;
>>>>> + const struct atmel_flex_caps *caps;
>>>>> struct resource *res;
>>>>> struct atmel_flexcom *ddata;
>>>>> int err;
>>>>> @@ -76,13 +82,60 @@ static int atmel_flexcom_probe(struct
>>>> platform_device *pdev)
>>>>> */
>>>>> writel(FLEX_MR_OPMODE(ddata->opmode), ddata->base +
>> FLEX_MR);
>>>>>
>>>>> + caps = of_device_get_match_data(&pdev->dev);
>>>>> + if (!caps) {
>>>>> + dev_err(&pdev->dev, "Could not retrieve flexcom caps\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + /* Flexcom Mux */
>>>>> + if (caps->has_flx_mux && of_property_read_bool(np, "mux-
>> controls"))
>>>> {
>>>>> + struct mux_control *flx_mux;
>>>>> + struct of_phandle_args args;
>>>>> + int i, count;
>>>>> +
>>>>> + flx_mux = devm_mux_control_get(&pdev->dev, NULL);
>>>>> + if (IS_ERR(flx_mux))
>>>>> + return PTR_ERR(flx_mux);
>>>>> +
>>>>> + count = of_property_count_strings(np, "mux-control-names");
>>>>> + for (i = 0; i < count; i++) {
>>>>> + err = of_parse_phandle_with_fixed_args(np, "mux-
>> controls",
>>>> 1, i, &args);
>>>>> + if (err)
>>>>> + break;
>>>>> +
>>>>> + err = mux_control_select(flx_mux, args.args[0]);
>>>>> + if (!err) {
>>>>> + mux_control_deselect(flx_mux);
>>>>
>>>> This is suspect. When you deselect the mux you rely on the mux to be
>>>> configured with "as-is" as the idle state. But you do not document that.
>>>> This is also fragile in that you silently rely on noone else selecting
>>>> the mux to some unwanted state behind your back (mux controls are not
>>>> exclusive the way e.g. gpio pins or pwms are). The protocol is that
>>>> others may get a ref to the mux control and select it as long as noone
>>>> else has selected it.
>>>>
>>>> The only sane thing to do is to keep the mux selected for the whole
>>>> duration when you rely on it to be in your desired state.
>>>>
>>>
>>> Yes, mux is kept selected until configuring register is done. Please see
>> below log where
>>> I added debug prints just for understanding:
>>> # dmesg | grep KK
>>> [ 0.779827] KK: Func: atmel_flexcom_probe ***** [START flx muxing]
>> ********
>>> [ 0.779875] KK: Func: atmel_flexcom_probe i = 0 args[0] = 0
>>> [ 0.779890] KK: Func: mux_control_select [Entered]
>>> [ 0.779902] KK: Func: mux_lan966x_set [Entered] state = 0
>>> [ 0.779977] KK: Func: mux_lan966x_set [Read] = 0x1fffef <<<----- setting
>> mux_lan966x[state].ss_pin "4" which is passed from dts
>>> [ 0.779992] KK: Func: mux_lan966x_set [Exit]
>>> [ 0.780002] KK: Func: mux_control_select [Exit]
>>> [ 0.780011] KK: Func: mux_control_deselect [Entered]
>>> [ 0.780021] KK: Func: mux_control_deselect [Exit]
>>
>> You misunderstand. The mux control is only "selected" between the call
>> to mux_control_select() and the following call to
>> mux_control_deselect().
>>
>> After that, the mux control is "idle". However, in your case the
>> "idle-state" is configured to "as-is" (which is the default), so the
>> mux control (naturally) remains in the previously selected state while
>> idle. But you are not documenting that limitation, and with this
>> implementation you *must* have a mux control that uses "as-is" as its
>> idle state. But that is an unexpected and broken limitation, and a
>> much better solution is to simply keep the mux control "selected" for
>> the complete duration when you rely on it to be in whatever state you
>> need it to be in.
>>
> I am new to mux drivers.
> Let me try to explain why I have mux_control_deselect if there is no err from mux_control_select()
> For example,
> 1. When I have one only chip-select CS0 for flexcom3 to be mapped to flexcom shared pin 9:
> As per previously shared log, FLEXCOM_SHARED[3]:SS_MASK[0] is being configured to expected value
> even before mux_control_deseletc().
> 2. When I have to map two chip-selects of flx3 - CS0 to flexcom shared 9
> CS1 to flexcom shared pin 7
> FLEXCOM_SHARED[3]:SS_MASK[0] is set to expected value 0x1fffef
> I see console hangs at mux_control_select() if I don’t call mux_control_deselect
> while mapping second chip-select FLEXCOM_SHARED[3]:SS_MASK[1].
> After reading below description from mux_control_select() :
> " * On successfully selecting the mux-control state, it will be locked until
> * there is a call to mux_control_deselect()."
> Following this help text, I called mux_control_deselect() if there is no error from mux_control_select()
> and then it worked. FLEXCOM_SHARED[3]:SS_MASK[1] is now set to expected value 0x1fffbf.
>
> Please explain me if I am missing something.
You should wait with the deselect until you need to change the state
of the mux. I.e., on init/probe you set some variable to -1, like so:
priv->mux_error = -1;
and when you later need to set/change the state of the mux, you do:
if (!priv->mux_error)
mux_control_deselect(...);
priv->mux_error = mux_control_select(...);
and then you cleanup at the end of life:
if (!priv->mux_error)
mux_control_deselect(...);
Or something like that.
The mux api is obviously not a good fit for the use case of long
lived selects like you appear to have here. The api was designed
for short lived selects, along the lines of:
int
so_something(...)
{
int err;
err = mux_control_select(...);
if (err)
return err;
do_it(...);
mux_control_deselect(...);
}
>
>>>>> + } else {
>>>>> + dev_err(&pdev->dev, "Failed to select FLEXCOM
>> mux\n");
>>>>> + return err;
>>>>> + }
>>>>> + }
>>>>> + }
>>>>> +
>>>>> clk_disable_unprepare(ddata->clk);
>>>>>
>>>>> return devm_of_platform_populate(&pdev->dev);
>>>>> }
>>>>>
>>>>> +static const struct atmel_flex_caps atmel_flexcom_caps = {};
>>>>> +
>>>>> +static const struct atmel_flex_caps lan966x_flexcom_caps = {
>>>>> + .has_flx_mux = true,
>>>>> +};
>>>>> +
>>>>> static const struct of_device_id atmel_flexcom_of_match[] = {
>>>>> - { .compatible = "atmel,sama5d2-flexcom" },
>>>>> + {
>>>>> + .compatible = "atmel,sama5d2-flexcom",
>>>>> + .data = &atmel_flexcom_caps,
>>>>> + },
>>>>> +
>>>>> + {
>>>>> + .compatible = "microchip,lan966-flexcom",
>>>>> + .data = &lan966x_flexcom_caps,
>>>>> + },
>>>>> +
>>>>> { /* sentinel */ }
>>>>> };
>>>>> MODULE_DEVICE_TABLE(of, atmel_flexcom_of_match);
>>>>> diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
>>>>> index e5c571fd232c..ea09f474bc2f 100644
>>>>> --- a/drivers/mux/Kconfig
>>>>> +++ b/drivers/mux/Kconfig
>>>>> @@ -45,6 +45,18 @@ config MUX_GPIO
>>>>> To compile the driver as a module, choose M here: the module will
>>>>> be called mux-gpio.
>>>>>
>>>>> +config MUX_LAN966
>>>>> + tristate "LAN966 Flexcom multiplexer"
>>>>> + depends on OF || COMPILE_TEST
>>>>> + help
>>>>> + Lan966 Flexcom Multiplexer controller.
>>>>> +
>>>>> + The driver supports mapping 2 chip-selects of each of the lan966
>>>>> + flexcoms to 21 flexcom shared pins.
>>>>> +
>>>>> + To compile the driver as a module, choose M here: the module will
>>>>> + be called mux-lan966.
>>>>> +
>>>>> config MUX_MMIO
>>>>> tristate "MMIO/Regmap register bitfield-controlled Multiplexer"
>>>>> depends on OF || COMPILE_TEST
>>>>> diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
>>>>> index 6e9fa47daf56..53a9840d96fa 100644
>>>>> --- a/drivers/mux/Makefile
>>>>> +++ b/drivers/mux/Makefile
>>>>> @@ -7,10 +7,12 @@ mux-core-objs := core.o
>>>>> mux-adg792a-objs := adg792a.o
>>>>> mux-adgs1408-objs := adgs1408.o
>>>>> mux-gpio-objs := gpio.o
>>>>> +mux-lan966-objs := lan966-flx.o
>>>>> mux-mmio-objs := mmio.o
>>>>>
>>>>> obj-$(CONFIG_MULTIPLEXER) += mux-core.o
>>>>> obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o
>>>>> obj-$(CONFIG_MUX_ADGS1408) += mux-adgs1408.o
>>>>> obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
>>>>> +obj-$(CONFIG_MUX_LAN966) += mux-lan966.o
>>>>> obj-$(CONFIG_MUX_MMIO) += mux-mmio.o
>>>>> diff --git a/drivers/mux/lan966-flx.c b/drivers/mux/lan966-flx.c
>>>>> new file mode 100644
>>>>> index 000000000000..2c7dab616a6a
>>>>> --- /dev/null
>>>>> +++ b/drivers/mux/lan966-flx.c
>>>>> @@ -0,0 +1,121 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>> +/*
>>>>> + * LAN966 Flexcom MUX driver
>>>>> + *
>>>>> + * Copyright (c) 2022 Microchip Inc.
>>>>> + *
>>>>> + * Author: Kavyasree Kotagiri <kavyasree.kotagiri@...rochip.com>
>>>>
>>>> Looks like it is based on mmio.c?
>>>>
>>> Yes, I took mmio.c driver as reference.
>>>
>>
>> So, then the above copyright and authorship info is not complete.
>>
>>>>> + */
>>>>> +
>>>>> +#include <linux/err.h>
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/of_platform.h>
>>>>> +#include <linux/platform_device.h>
>>>>> +#include <linux/property.h>
>>>>> +#include <linux/mux/driver.h>
>>>>> +#include <linux/io.h>
>>>>> +
>>>>> +#define FLEX_SHRD_MASK 0x1FFFFF
>>>>> +#define LAN966_MAX_CS 21
>>>>> +
>>>>> +static void __iomem *flx_shared_base;
>>>>
>>>> I would much prefer to store the combined address (base+offset)
>>>> in the mux private data instead of only storing the offset and
>>>> then unnecessarily rely on some piece of global state (that
>>>> will be clobbered by other instances).
>>>>
>>> Ok. I will try to see if this is relevant and change accordingly.
>>>
>>>>> +struct mux_lan966x {
>>>>
>>>> Why is the file named lan966, but then everything inside lan966x?
>>>>
>>>>> + u32 offset;
>>>>> + u32 ss_pin;
>>>>> +};
>>>>> +
>>>>> +static int mux_lan966x_set(struct mux_control *mux, int state)
>>>>> +{
>>>>> + struct mux_lan966x *mux_lan966x = mux_chip_priv(mux->chip);
>>>>> + u32 val;
>>>>> +
>>>>> + val = ~(1 << mux_lan966x[state].ss_pin) & FLEX_SHRD_MASK;
>>>>> + writel(val, flx_shared_base + mux_lan966x[state].offset);
>>>>
>>>> This reads memory you have not allocated, if you select a state
>>>> other than zero.
>>>>
>>> Ok. I will return error condition in case of trying to access none existing
>> entry.
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static const struct mux_control_ops mux_lan966x_ops = {
>>>>> + .set = mux_lan966x_set,
>>>>> +};
>>>>> +
>>>>> +static const struct of_device_id mux_lan966x_dt_ids[] = {
>>>>> + { .compatible = "microchip,lan966-flx-mux", },
>>>>> + { /* sentinel */ }
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(of, mux_lan966x_dt_ids);
>>>>> +
>>>>> +static int mux_lan966x_probe(struct platform_device *pdev)
>>>>> +{
>>>>> + struct device_node *np = pdev->dev.of_node;
>>>>> + struct device *dev = &pdev->dev;
>>>>> + struct mux_lan966x *mux_lan966x;
>>>>> + struct mux_chip *mux_chip;
>>>>> + int ret, num_fields, i;
>>>>> +
>>>>> + ret = of_property_count_u32_elems(np, "mux-offset-pin");
>>>>> + if (ret == 0 || ret % 2)
>>>>> + ret = -EINVAL;
>>>>> + if (ret < 0)
>>>>> + return dev_err_probe(dev, ret,
>>>>> + "mux-offset-pin property missing or invalid");
>>>>> + num_fields = ret / 2;
>>>>> +
>>>>> + mux_chip = devm_mux_chip_alloc(dev, num_fields,
>>>> sizeof(*mux_lan966x));
>>>>
>>>> I might be thoroughly mistaken and confused by the code, but it seems
>>>> very strange that a subsequenct select is not undoing what a previous
>>>> select once did. Each state seems to write to its own register offset,
>>>> and there is nothing that restores the first register offset with you
>>>> switch states.
>>>>
>>>> Care to explain how muxing works and what you are expected to do to
>>>> manipulate the mux? Is there some link to public documentation? I did
>>>> a quick search for lan966 but came up with nothing relevant.
>>>>
>>> LAN966 has 5 flexcoms(which can be used as USART/SPI/I2C interface).
>>> FLEXCOM has two chip-select I/O lines namely CS0 and CS1
>>> in SPI mode, CTS and RTS in USART mode. These FLEXCOM pins are
>> optional.
>>> These chip-selects can be mapped to flexcom shared pin [0-21] which can
>> be
>>> done by configuring flexcom multiplexer register(FLEXCOM_SHARED[0-
>> 4]:SS_MASK[0-1])
>>> Driver explanation:
>>> "flx_shared_base" is used to get base address of Flexcom shared
>> multiplexer
>>> "mux-offset-pin" property is used to get cs0/cs1 offset and flexcom shared
>> pin[0-21] of a flexcom.
>>> state value passed from atmel-flexcom is used to configure respective
>>> FLEXCOM_SHARED[0-4]:SS_MASK[0-1] register with offset and flexcom
>> shared pin.
>>
>> Ok, let me try to interpret that. You wish enable a "fan out" of these
>> two chip-selects for any of the 5 flexcoms (in SPI mode?) such that
>> these 10 internal chip-selects can be connected to any of 21 external
>> pins?
>>
>> If that's correct and if you wish to interface with e.g. 20 chips,
>> then it would be possible to have 20 states for one mux control and
>> then reach the 20 chips using only CS0 from FLEXCOM0, or it would be
>> possible to have 2 states for 10 mux controls, one each for CS0/CS1 of
>> all five flexcoms and also reach 20 chips. Or any wild combo you
>> imagine is useful.
>>
>> Is that correctly understood?
>>
>> Assuming so, then you can have a maximum of 10 mux controls, and for
>> each mux control you need a variable number of legitimate states. Each
>> mux control would also need to know at what address/offset its SS_MASK
>> register is at and what pins/states are valid.
>>
>> But your code does not implemnent the above. You allocate num_fields
>> mux controls, which is what confuses the hell out of me. num_fields is
>> the number of states, not the number of mux controls! And you also
>> need to specify an individual offset for each state, and that makes no
>> sense at all, at least not to me.
>>
>> So, instead, I think you want to have:
>>
>> struct mux_lan966x {
>> u32 ss_mask;
>> int num_pins;
>> u32 ss_pin[];
>> };
>>
>> And then do:
>>
>> mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux_lan966x) +
>> num_pins * sizeof(u32));
>>
>> (or however that size is best spelled, I haven't kept up)
>>
>> The set operation would be along the lines of:
>>
>> static int mux_lan966x_set(struct mux_control *mux, int state)
>> {
>> struct mux_lan966x *mux_lan966x = mux_chip_priv(mux->chip);
>> u32 val;
>>
>> if (state < 0 || state >= mux_lan966x->num_pins)
>> return -1;
>>
>> val = ~(1 << mux_lan966x->ss_pin[state]) & FLEX_SHRD_MASK;
>> writel(val, flx_shared_base + mux_lan966x->ss_mask);
>>
>> return 0;
>> }
>>
>> Because, one mux control should only ever need to know about one offset,
>> as it should only ever write to its own SS_MASK register. And you will
>> need some private space such that you can keep track of which states
>> are legit.
>>
>> I would also separate out the SS_MASK offset from the mux-offset-pin
>> property and have one property for that value and then a straight
>> array for the valid pin numbers in another property (no longer named
>> mux-offset-pin of course).
>>
>> Or something like that and assuming I understand how the FLEXCOMs work
>> and what you want to do etc.
>>
>
> Thank you for your comments
> I agree, I will change number of mux controls to 1 in devm_mux_chip_alloc()
> I would like to use mux-offset-pin property because
> each chip-select must be mapped to a unique flexcom shared pin.
> For example,
> mux-offset-pin = <0x18 9>, /* 0: flexcom3 CS0 mapped to pin-9 */
> <0x1c 7>, /* 1: flexcom3 CS1 mapped to pin-7 */
> <0x20 4>; /* 2: flexcom4 CS0 mapped to pin-4 */};
> I want to use mux-offset-pin property to be clear about which offset is mapped to which
> flexcom shared pin. Here, 0, 1, 2.. entries represents state passed from mux_control_select(mux, state).
>
> Please provide your comments.
It seems you want to just use the static info from the devicetree
to program your registers once and for all? That's not a problem
that a mux driver is aming at solving. My thought was that if you
have one SPI device with chip select on pin-7, another with chip-
select on pin-9 and a third on pin-4, then you /could/ use them all
from e.g. flexcom3/CS0 by reprogramming that SS_MASK register
at the time you want to access one of the three chips. You could
then of course not access the three chips in parallel, but the
muxing of the accesses to three chips like that is the problem
that the mux subsystem helps with.
I.e. pretty much exactly as is described in
Documentation/devicetree/bindings/spi/spi-mux.yaml
but with a mux node bringing your new driver into the picture
instead of the "gpio-mux" in that example (and the "CS-X" signal
and the "Mux" block in that picture would be internal to the SoC).
If you are content with just programming a fixed set of values to
a couple of registers depending on how the board is wired, some
extra DT property on some node related to the flexcom seems like
a better fit than a mux driver.
Having said that, the mux solution above would solve this static
case too. You would just need to configure a mux with one state.
It would be a couple of extra nodes in the device tree, but it
would certainly work (and also cover the more complex case when
someone pops up and needs that).
Cheers,
Peter
>> Cheers,
>> Peter
>>
>>
>>>>> + if (IS_ERR(mux_chip))
>>>>> + return dev_err_probe(dev, PTR_ERR(mux_chip),
>>>>> + "failed to allocate mux_chips\n");
>>>>> +
>>>>> + mux_lan966x = mux_chip_priv(mux_chip);
>>>>> +
>>>>> + flx_shared_base =
>> devm_platform_get_and_ioremap_resource(pdev,
>>>> 0, NULL);
>>>>> + if (IS_ERR(flx_shared_base))
>>>>> + return dev_err_probe(dev, PTR_ERR(flx_shared_base),
>>>>> + "failed to get flexcom shared base address\n");
>>>>> +
>>>>> + for (i = 0; i < num_fields; i++) {
>>>>> + struct mux_control *mux = &mux_chip->mux[i];
>>>>> + u32 offset, shared_pin;
>>>>> +
>>>>> + ret = of_property_read_u32_index(np, "mux-offset-pin",
>>>>> + 2 * i, &offset);
>>>>> + if (ret == 0)
>>>>> + ret = of_property_read_u32_index(np, "mux-offset-pin",
>>>>> + 2 * i + 1,
>>>>> + &shared_pin);
>>>>> + if (ret < 0)
>>>>> + return dev_err_probe(dev, ret,
>>>>> + "failed to read mux-offset-pin property: %d", i);
>>>>> +
>>>>> + if (shared_pin >= LAN966_MAX_CS)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + mux_lan966x[i].offset = offset;
>>>>> + mux_lan966x[i].ss_pin = shared_pin;
>>>>
>>>> This clobbers memory you have not allocated, if num_fields >= 1.
>>>>
>>>> Cheers,
>>>> Peter
>>>>
>>>>> +
>>>>> + mux->states = LAN966_MAX_CS;
>>>>> + }
>>>>> +
>>>>> + mux_chip->ops = &mux_lan966x_ops;
>>>>> +
>>>>> + ret = devm_mux_chip_register(dev, mux_chip);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static struct platform_driver mux_lan966x_driver = {
>>>>> + .driver = {
>>>>> + .name = "lan966-mux",
>>>>> + .of_match_table = of_match_ptr(mux_lan966x_dt_ids),
>>>>> + },
>>>>> + .probe = mux_lan966x_probe,
>>>>> +};
>>>>> +
>>>>> +module_platform_driver(mux_lan966x_driver);
>>>>> +
>>>>> +MODULE_DESCRIPTION("LAN966 Flexcom multiplexer driver");
>>>>> +MODULE_AUTHOR("Kavyasree Kotagiri
>>>> <kavyasree.kotagiri@...rochip.com>");
>>>>> +MODULE_LICENSE("GPL v2");
>>>>> +
Powered by blists - more mailing lists