On Wed, 29 Mar 2023 at 07:00, Michal Simek michal.simek@amd.com wrote:
On 3/27/23 23:15, jassisinghbrar@gmail.com wrote:
diff --git a/drivers/fwu-mdata/raw_mtd.c b/drivers/fwu-mdata/raw_mtd.c new file mode 100644 index 0000000000..4b1a10073a --- /dev/null +++ b/drivers/fwu-mdata/raw_mtd.c @@ -0,0 +1,272 @@ +// SPDX-License-Identifier: GPL-2.0+
Just a note: Did you choose GPL-2.0+ by purpose? Or it is just c&p?
just c&p. though isn't that the same as GPL-2.0-or-later ?
.......
+extern struct fwu_mtd_image_info fwu_mtd_images[];
if there is a need to share it. It should go to header.
include/fwu,h would be the header. Which is supposed to be very global, and doesn't seem very appropriate to mention private data shared between a driver and helper library. If people still insist, I will make the change.
And fwu_mtd_images[] is not defined when only this patch is applied. It means order of patches is not right. 1/6 and 2/6 should be swapped
I think 1 and 2 should be merged rather.
......
if (!mtd_priv->mtd) {log_err("Failed to find mtd device by fwu-mdata-store\n");return -ENODEV;}/* Get the offset of primary and seconday mdata */typo
ok
......
+static const struct udevice_id fwu_mdata_ids[] = {
{ .compatible = "u-boot,fwu-mdata-mtd" },{ }+};
As I said this DT binding should be approved first to make sure that we don't need to fix DT binding in future. Just simply do it right from the begining.
Yes, I will cc Rob in the next submission (I only forgot last time). However, let us note that fwu-mdata-gpt.yaml isn't blessed either. I am not sure if there is any reason for the fwu node to even be in the dts for kernel. But sure it is good to have it eyeballed by the DT gods.
cheers.