> > +#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;
>
> Why do you have file-scope shared variable? Cannot it be passed via
> private data?
>
I want flx_shared_base to be global variable and use struct mux_lan966x to represent only
"mux-offset-pin" parameters.
> > +struct mux_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);
> > +
> > + 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));
> > + 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;
> > +
> > + 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),
>
> of_match_ptr comes with maybe_unused on data structure. Are you sure it
> does not have W=1 warnings during compile tests? Just drop the
> of_match_ptr.
>
No, I haven't noticed any warning. Other mux drivers also follow the same.
> > + },
> > + .probe = mux_lan966x_probe,
> > +};
> > +
> > +module_platform_driver(mux_lan966x_driver);
>
> Missing MODULE() stuff.
Ok. I will add it in next version of patch series.
>
>
> Best regards,
> Krzysztof
