On 11/08/17 15:15, Rob Herring wrote:
> +Bjorn
>
> On Fri, Aug 11, 2017 at 8:30 AM, Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
>> Since "/firmware" does not have its own "compatible" property as it's
>> just collection of nodes representing firmware interface, it's sub-nodes
>> are not populated during system initialization.
>>
>> Currently different firmware drivers search the /firmware/ node and
>> populate the sub-node devices selectively. Instead we can populate
>> the /firmware/ node during init to avoid more drivers continuing to
>> populate the devices selectively.
>>
>> This patch adds initcall to achieve the same.
>>
>> Cc: Arnd Bergmann <arnd@xxxxxxxx>
>> Cc: Rob Herring <robh@xxxxxxxxxx>
>> Signed-off-by: Sudeep Holla <sudeep.holla@xxxxxxx>
>> ---
>> drivers/firmware/Makefile | 1 +
>> drivers/firmware/of.c | 34 ++++++++++++++++++++++++++++++++++
>> 2 files changed, 35 insertions(+)
>> create mode 100644 drivers/firmware/of.c
>>
>> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
>> index 91d3ff62c653..d9a6fce43613 100644
>> --- a/drivers/firmware/Makefile
>> +++ b/drivers/firmware/Makefile
>> @@ -17,6 +17,7 @@ obj-$(CONFIG_ISCSI_IBFT) += iscsi_ibft.o
>> obj-$(CONFIG_FIRMWARE_MEMMAP) += memmap.o
>> obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
>> obj-$(CONFIG_FW_CFG_SYSFS) += qemu_fw_cfg.o
>> +obj-$(CONFIG_OF) += of.o
>> obj-$(CONFIG_QCOM_SCM) += qcom_scm.o
>> obj-$(CONFIG_QCOM_SCM_64) += qcom_scm-64.o
>> obj-$(CONFIG_QCOM_SCM_32) += qcom_scm-32.o
>> diff --git a/drivers/firmware/of.c b/drivers/firmware/of.c
>> new file mode 100644
>> index 000000000000..149b9660fb44
>> --- /dev/null
>> +++ b/drivers/firmware/of.c
>> @@ -0,0 +1,34 @@
>> +/*
>> + * Populates the nodes under /firmware/ device tree node
>> + *
>> + * Copyright (C) 2017 ARM Ltd.
>> + *
>> + * This file is subject to the terms and conditions of the GNU General Public
>> + * License. See the file "COPYING" in the main directory of this archive
>> + * for more details.
>> + *
>> + * Released under the GPLv2 only.
>
> Why do you have both the above 2 paragraphs and an SPDX tag? I'd drop
> the above (unless your lawyers told you otherwise).
>
I thought so initially but then just copied from existing file
(drivers/base/arch_topology.c in this case)
>> + * SPDX-License-Identifier: GPL-2.0
>> + */
>> +
>> +#include <linux/init.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +
>> +static int __init firmware_of_init(void)
>
> I'd prefer this is added to of_platform_default_populate_init() and
> handled like /reserved-memory.
>
Yes that was my other option, wanted to check in the cover letter but
forgot.
> But be aware that Bjorn is reworking this function[1].
>
OK, thanks for the pointers.
>> +{
>> + struct device_node *fw_np;
>> + int ret;
>> +
>> + fw_np = of_find_node_by_name(NULL, "firmware");
>
> This matches any node named firmware. I see a few instances like RPi
> that are not /firmware (though RPi we can move probably).
> of_find_node_by_path would be safer.
>
OK
>> +
>> + if (!fw_np)
>> + return 0;
>> +
>> + ret = of_platform_populate(fw_np, NULL, NULL, NULL);
>> +
>> + of_node_put(fw_np);
>> +
>> + return ret;
>> +}
>> +arch_initcall_sync(firmware_of_init);
>
> You perhaps missed a few instances. "amlogic,meson-gxbb-sm" looks like
> it could be converted to a driver.
Indeed, was looking for the term "firmware" only :(, will add.
"tlm,trusted-foundations" appears
> to be needed early for bringing up cores, so it probably can't change.
That was my intially understand too when I looked at the way probe was
handled, but then I see it's module_init, so I went ahead with this change.
> They don't have to be converted as part of this series though.
>
OK
--
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
