> ...
>
> > +struct intel_dg_nvm {
> > + struct kref refcnt;
> > + void __iomem *base;
> > + size_t size;
> > + unsigned int nregions;
> > + struct {
> > + const char *name;
> > + u8 id;
> > + u64 offset;
> > + u64 size;
> > + } regions[];
>
> __counted_by(nregions)?
>
Sure, will add
> > +};
> > +
> > +static void intel_dg_nvm_release(struct kref *kref)
> > +{
> > + struct intel_dg_nvm *nvm = container_of(kref, struct intel_dg_nvm,
> refcnt);
> > + int i;
> > +
> > + pr_debug("freeing intel_dg nvm\n");
> > + for (i = 0; i < nvm->nregions; i++)
> > + kfree(nvm->regions[i].name);
> > + kfree(nvm);
> > +}
> > +
> > +static int intel_dg_mtd_probe(struct auxiliary_device *aux_dev,
> > + const struct auxiliary_device_id *aux_dev_id)
> > +{
> > + struct intel_dg_nvm_dev *invm =
> auxiliary_dev_to_intel_dg_nvm_dev(aux_dev);
> > + struct device *device;
> > + struct intel_dg_nvm *nvm;
> > + unsigned int nregions;
> > + unsigned int i, n;
> > + size_t size;
> > + char *name;
> > + int ret;
> > +
> > + device = &aux_dev->dev;
> > +
> > + /* count available regions */
> > + for (nregions = 0, i = 0; i < INTEL_DG_NVM_REGIONS; i++) {
> > + if (invm->regions[i].name)
> > + nregions++;
> > + }
> > +
> > + if (!nregions) {
> > + dev_err(device, "no regions defined\n");
> > + return -ENODEV;
> > + }
> > +
> > + size = sizeof(*nvm) + sizeof(nvm->regions[0]) * nregions;
>
> struct_size()? (and maybe no need for size)
>
Will do, thanks for suggestion
> > + nvm = kzalloc(size, GFP_KERNEL);
> > + if (!nvm)
> > + return -ENOMEM;
> > +
> > + kref_init(&nvm->refcnt);
> > +
> > + nvm->nregions = nregions;
> > + for (n = 0, i = 0; i < INTEL_DG_NVM_REGIONS; i++) {
> > + if (!invm->regions[i].name)
> > + continue;
> > +
> > + name = kasprintf(GFP_KERNEL, "%s.%s",
> > + dev_name(&aux_dev->dev), invm-
> >regions[i].name);
> > + if (!name)
> > + continue;
> > + nvm->regions[n].name = name;
> > + nvm->regions[n].id = i;
> > + n++;
> > + }
>
> Should we set the exact number of regions, should a kasprintf() fail?
> nvm->nregions = n;
> This would make __counted_by be more accurate in its check.
>
Sounds good, will do
> > +
> > + nvm->base = devm_ioremap_resource(device, &invm->bar);
> > + if (IS_ERR(nvm->base)) {
> > + dev_err(device, "mmio not mapped\n");
> > + ret = PTR_ERR(nvm->base);
> > + goto err;
> > + }
> > +
> > + dev_set_drvdata(&aux_dev->dev, nvm);
> > +
> > + return 0;
> > +
> > +err:
> > + kref_put(&nvm->refcnt, intel_dg_nvm_release);
> > + return ret;
> > +}
>
> ...
>
> CJ
- -
Thanks,
Sasha
