Messages in this thread | | | Date | Wed, 3 Aug 2022 17:30:21 +0200 | From | Greg KH <> | Subject | Re: [PATCH 2/2] virt: vmgenid: add support for generation counter |
| |
On Wed, Aug 03, 2022 at 05:21:27PM +0200, bchalios@amazon.es wrote: > +static const struct file_operations fops = { > + .owner = THIS_MODULE, > + .open = vmgenid_open, > + .read = vmgenid_read, > + .mmap = vmgenid_mmap, > + .llseek = noop_llseek,
Where is this new user/kernel api being documented?
See, put it in the code please, no one knows to look in a random file in Documentation/
> +}; > + > +static struct miscdevice vmgenid_misc = { > + .minor = MISC_DYNAMIC_MINOR, > + .name = "vmgenid", > + .fops = &fops, > }; > > static int parse_vmgenid_address(struct acpi_device *device, acpi_string object_name, > @@ -57,7 +124,7 @@ static int vmgenid_add(struct acpi_device *device) > phys_addr_t phys_addr; > int ret; > > - state = devm_kmalloc(&device->dev, sizeof(*state), GFP_KERNEL); > + state = devm_kzalloc(&device->dev, sizeof(*state), GFP_KERNEL); > if (!state) > return -ENOMEM; > > @@ -74,6 +141,27 @@ static int vmgenid_add(struct acpi_device *device) > > device->driver_data = state; > > + /* Backwards compatibility. If CTRA is not there we just don't expose > + * the char device > + */ > + ret = parse_vmgenid_address(device, "CTRA", &state->gen_cntr_addr); > + if (ret) > + return 0; > + > + state->next_counter = devm_memremap(&device->dev, state->gen_cntr_addr, > + sizeof(u32), MEMREMAP_WB); > + if (IS_ERR(state->next_counter)) > + return 0; > + > + memcpy(&state->misc, &vmgenid_misc, sizeof(state->misc)); > + ret = misc_register(&state->misc); > + if (ret) { > + devm_memunmap(&device->dev, state->next_counter); > + return 0;
Why are you not returning an error? Why is this ok?
And why call devm_memunmap() directly? That kind of defeats the purpose of using devm_memremap(), right?
thanks,
greg k-h
| |