Re: [PATCH v2 11/11] RAS: add DeviceTree firmware-first CPER provider

[Ahmed Tiba <ahmed.tiba@arm.com>]

On 21/02/2026 09:06, Krzysztof Kozlowski wrote:
> On 20/02/2026 14:42, Ahmed Tiba wrote:
>> +	rc = ghes_ffh_init_ack(pdev, ctx);
>> +	if (rc)
>> +		return rc;
>> +
>> +	rc = ghes_ffh_init_pool();
>> +	if (rc)
>> +		return rc;
>> +
>> +	ctx->estatus = devm_kzalloc(&pdev->dev, ctx->status_len, GFP_KERNEL);
>> +	if (!ctx->estatus)
>> +		return -ENOMEM;
>> +
>> +	ctx->generic = devm_kzalloc(&pdev->dev, sizeof(*ctx->generic), GFP_KERNEL);
>> +	if (!ctx->generic)
>> +		return -ENOMEM;
>> +
>> +	ctx->generic->header.type = ACPI_HEST_TYPE_GENERIC_ERROR;
>> +	ctx->generic->header.source_id =
>> +		atomic_inc_return(&ghes_ffh_source_ids);
>> +	ctx->generic->notify.type = ctx->sync ?
>> +		ACPI_HEST_NOTIFY_SEA : ACPI_HEST_NOTIFY_EXTERNAL;
>> +	ctx->generic->error_block_length = ctx->status_len;
>> +
>> +	ctx->irq = platform_get_irq_optional(pdev, 0);
> 
> Please read the kerneldoc - wrong check in if.

Got it. I’ll follow the kerneldoc: use `if (irq < 0) return irq;`.

>> +	if (ctx->irq <= 0) {
>> +		if (ctx->irq == -EPROBE_DEFER)
>> +			return ctx->irq;
>> +		dev_err(&pdev->dev, "interrupt is required (%d)\n", ctx->irq);
>> +		return -EINVAL;
>> +	}
>> +
>> +	rc = devm_request_threaded_irq(&pdev->dev, ctx->irq,
>> +				       NULL, ghes_ffh_irq,
>> +				       IRQF_ONESHOT,
>> +				       dev_name(&pdev->dev), ctx);
>> +	if (rc)
>> +		return rc;
>> +
>> +	platform_set_drvdata(pdev, ctx);
>> +	dev_info(&pdev->dev, "Firmware-first CPER status provider (interrupt)\n");
> 
> This does not look like useful printk message. Drivers should be silent
> on success:
> https://elixir.bootlin.com/linux/v6.15-rc7/source/Documentation/process/coding-style.rst#L913
> https://elixir.bootlin.com/linux/v6.15-rc7/source/Documentation/process/debugging/driver_development_debugging_guide.rst#L79

I will drop it.

> 
>> +	return 0;
>> +}
>> +
>> +static void ghes_ffh_remove(struct platform_device *pdev)
>> +{
>> +}
>> +
>> +static const struct of_device_id ghes_ffh_of_match[] = {
>> +	{ .compatible = "arm,ras-ffh" },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, ghes_ffh_of_match);
>> +
>> +static struct platform_driver ghes_ffh_driver = {
>> +	.driver = {
>> +		.name = "esource-dt",
>> +		.of_match_table = ghes_ffh_of_match,
>> +	},
>> +	.probe = ghes_ffh_probe,
>> +	.remove = ghes_ffh_remove,
>> +};
>> +
>> +module_platform_driver(ghes_ffh_driver);
>> +
>> +MODULE_AUTHOR("Ahmed Tiba <ahmed.tiba@arm.com>");
>> +MODULE_DESCRIPTION("Firmware-first CPER provider for DeviceTree platforms");
>> +MODULE_LICENSE("GPL");

Best regards,
Tiba