| Message ID | 20240314070433.4151931-3-adeep@lexina.in (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | soc: amlogic: add new meson-gx-socinfo-sm driver | expand |
On 14/03/2024 07:59, Viacheslav Bocharov wrote: > Amlogic SoCs have a SoC information secure-monitor call for SoC type, > package type, revision information and chipid. > This patchs adds support for secure-monitor call decoding and exposing > with the SoC bus infrastructure in addition to the previous SoC > Information driver. > > Signed-off-by: Viacheslav Bocharov <adeep@lexina.in> > --- > drivers/soc/amlogic/Kconfig | 10 ++ > drivers/soc/amlogic/Makefile | 1 + > drivers/soc/amlogic/meson-gx-socinfo-sm.c | 192 ++++++++++++++++++++++ > 3 files changed, 203 insertions(+) > create mode 100644 drivers/soc/amlogic/meson-gx-socinfo-sm.c > > diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig > index d08e398bdad4..82fc77ca3b4b 100644 > --- a/drivers/soc/amlogic/Kconfig > +++ b/drivers/soc/amlogic/Kconfig > @@ -26,6 +26,16 @@ config MESON_GX_SOCINFO > Say yes to support decoding of Amlogic Meson GX SoC family > information about the type, package and version. > > +config MESON_GX_SOCINFO_SM > + bool "Amlogic Meson GX SoC Information driver via Secure Monitor" > + depends on (ARM64 && ARCH_MESON || COMPILE_TEST) && MESON_SM=y > + default ARCH_MESON && MESON_SM > + select SOC_BUS > + help > + Say yes to support decoding of Amlogic Meson GX SoC family > + information about the type, package and version via secure > + monitor call. > + I wonder, why do you need socinfo driver per each SoC? Usually it is one or two per entire arch. > + > +static int meson_gx_socinfo_sm_probe(struct platform_device *pdev) > +{ > + struct soc_device_attribute *soc_dev_attr; > + struct soc_device *soc_dev; > + struct device_node *sm_np; > + struct meson_sm_firmware *fw; > + struct regmap *regmap; > + union meson_cpu_id socinfo; > + struct device *dev; > + int ret; > + > + /* check if chip-id is available */ > + if (!of_property_read_bool(pdev->dev.of_node, "amlogic,has-chip-id")) > + return -ENODEV; > + > + /* node should be a syscon */ > + regmap = syscon_node_to_regmap(pdev->dev.of_node); > + if (IS_ERR(regmap)) { > + dev_err(&pdev->dev, "failed to get regmap\n"); Syntax is: return dev_err_probe() > + return -ENODEV; Anyway wrong return code, use the real one you got. > + } > + > + sm_np = of_parse_phandle(pdev->dev.of_node, "secure-monitor", 0); > + if (!sm_np) { > + dev_err(&pdev->dev, "no secure-monitor node found\n"); > + return -ENODEV; -EINVAL > + } > + > + fw = meson_sm_get(sm_np); > + of_node_put(sm_np); > + if (!fw) { > + dev_info(&pdev->dev, "secure-monitor device not ready, probe later\n"); No, you never print messages on deferred probe. > + return -EPROBE_DEFER; > + } > + > + ret = regmap_read(regmap, AO_SEC_SOCINFO_OFFSET, &socinfo.raw); > + if (ret < 0) > + return ret; > + > + if (!socinfo.raw) { > + dev_err(&pdev->dev, "invalid regmap chipid value\n"); > + return -EINVAL; > + } > + > + soc_dev_attr = devm_kzalloc(&pdev->dev, sizeof(*soc_dev_attr), > + GFP_KERNEL); > + if (!soc_dev_attr) > + return -ENOMEM; > + > + soc_dev_attr->serial_number = socinfo_get_chipid(&pdev->dev, fw, &socinfo); > + > + soc_dev_attr->family = "Amlogic Meson"; > + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x:%x - %x:%x", > + socinfo.v1.major_id, > + socinfo.v1.chip_rev, > + socinfo.v1.pack_id, > + (socinfo.v1.reserved<<4) + socinfo.v1.layout_ver); > + soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%s (%s)", > + socinfo_v1_to_soc_id(socinfo), > + socinfo_v1_to_package_id(socinfo)); > + > + soc_dev = soc_device_register(soc_dev_attr); > + > + > + if (IS_ERR(soc_dev)) { > + kfree(soc_dev_attr->revision); > + kfree_const(soc_dev_attr->soc_id); > + kfree(soc_dev_attr); That's a double free. This was not tested. > + return PTR_ERR(soc_dev); > + } > + > + dev = soc_device_to_device(soc_dev); > + platform_set_drvdata(pdev, soc_dev); > + > + dev_info(dev, "Amlogic Meson %s Revision %x:%x (%x:%x) Detected (SM)\n", > + soc_dev_attr->soc_id, > + socinfo.v1.major_id, > + socinfo.v1.chip_rev, > + socinfo.v1.pack_id, > + (socinfo.v1.reserved<<4) + socinfo.v1.layout_ver); > + > + return PTR_ERR_OR_ZERO(dev); > +} > + > + > +static int meson_gx_socinfo_sm_remove(struct platform_device *pdev) > +{ > + struct soc_device *soc_dev = platform_get_drvdata(pdev); > + > + soc_device_unregister(soc_dev); If you free the memory in probe() error path, why you did not decide to free it here as well? It is symmetrical, so this should make you wonder - error path is wrong. Best regards, Krzysztof
Hi! Thanks for review. 14/03/2024 10.11, Krzysztof Kozlowski wrote: > On 14/03/2024 07:59, Viacheslav Bocharov wrote: >> Amlogic SoCs have a SoC information secure-monitor call for SoC type, >> package type, revision information and chipid. >> This patchs adds support for secure-monitor call decoding and exposing >> with the SoC bus infrastructure in addition to the previous SoC >> Information driver. >> >> Signed-off-by: Viacheslav Bocharov <adeep@lexina.in> >> --- >> drivers/soc/amlogic/Kconfig | 10 ++ >> drivers/soc/amlogic/Makefile | 1 + >> drivers/soc/amlogic/meson-gx-socinfo-sm.c | 192 ++++++++++++++++++++++ >> 3 files changed, 203 insertions(+) >> create mode 100644 drivers/soc/amlogic/meson-gx-socinfo-sm.c >> >> diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig >> index d08e398bdad4..82fc77ca3b4b 100644 >> --- a/drivers/soc/amlogic/Kconfig >> +++ b/drivers/soc/amlogic/Kconfig >> @@ -26,6 +26,16 @@ config MESON_GX_SOCINFO >> Say yes to support decoding of Amlogic Meson GX SoC family >> information about the type, package and version. >> >> +config MESON_GX_SOCINFO_SM >> + bool "Amlogic Meson GX SoC Information driver via Secure Monitor" >> + depends on (ARM64 && ARCH_MESON || COMPILE_TEST) && MESON_SM=y >> + default ARCH_MESON && MESON_SM >> + select SOC_BUS >> + help >> + Say yes to support decoding of Amlogic Meson GX SoC family >> + information about the type, package and version via secure >> + monitor call. >> + > > I wonder, why do you need socinfo driver per each SoC? Usually it is one > or two per entire arch. We use this driver for GX and almost all newer SoCs from AmLogic (similar to the original meson-gx-socinfo). In the fourth patch, this driver is specifically enabled for the GX, G12 (g12a, g12b, sm1), AXG, A1 families. > >> + >> +static int meson_gx_socinfo_sm_probe(struct platform_device *pdev) >> +{ >> + struct soc_device_attribute *soc_dev_attr; >> + struct soc_device *soc_dev; >> + struct device_node *sm_np; >> + struct meson_sm_firmware *fw; >> + struct regmap *regmap; >> + union meson_cpu_id socinfo; >> + struct device *dev; >> + int ret; >> + >> + /* check if chip-id is available */ >> + if (!of_property_read_bool(pdev->dev.of_node, "amlogic,has-chip-id")) >> + return -ENODEV; >> + >> + /* node should be a syscon */ >> + regmap = syscon_node_to_regmap(pdev->dev.of_node); >> + if (IS_ERR(regmap)) { >> + dev_err(&pdev->dev, "failed to get regmap\n"); > > Syntax is: > > return dev_err_probe() > >> + return -ENODEV; > > Anyway wrong return code, use the real one you got. > Thanks! I'l fix all dev_err&return calls to this helper. >> + } >> + >> + sm_np = of_parse_phandle(pdev->dev.of_node, "secure-monitor", 0); >> + if (!sm_np) { >> + dev_err(&pdev->dev, "no secure-monitor node found\n"); >> + return -ENODEV; > > -EINVAL Fixed. > >> + } >> + >> + fw = meson_sm_get(sm_np); >> + of_node_put(sm_np); >> + if (!fw) { >> + dev_info(&pdev->dev, "secure-monitor device not ready, probe later\n"); > > No, you never print messages on deferred probe. Fixed to dev_dbg like in dev_err_probe. > >> + return -EPROBE_DEFER; >> + } >> + >> + ret = regmap_read(regmap, AO_SEC_SOCINFO_OFFSET, &socinfo.raw); >> + if (ret < 0) >> + return ret; >> + >> + if (!socinfo.raw) { >> + dev_err(&pdev->dev, "invalid regmap chipid value\n"); >> + return -EINVAL; >> + } >> + >> + soc_dev_attr = devm_kzalloc(&pdev->dev, sizeof(*soc_dev_attr), >> + GFP_KERNEL); >> + if (!soc_dev_attr) >> + return -ENOMEM; >> + >> + soc_dev_attr->serial_number = socinfo_get_chipid(&pdev->dev, fw, &socinfo); >> + >> + soc_dev_attr->family = "Amlogic Meson"; >> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x:%x - %x:%x", >> + socinfo.v1.major_id, >> + socinfo.v1.chip_rev, >> + socinfo.v1.pack_id, >> + (socinfo.v1.reserved<<4) + socinfo.v1.layout_ver); >> + soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%s (%s)", >> + socinfo_v1_to_soc_id(socinfo), >> + socinfo_v1_to_package_id(socinfo)); >> + >> + soc_dev = soc_device_register(soc_dev_attr); >> + >> + >> + if (IS_ERR(soc_dev)) { >> + kfree(soc_dev_attr->revision); >> + kfree_const(soc_dev_attr->soc_id); >> + kfree(soc_dev_attr); > > That's a double free. This was not tested. Please, describe the problem. I don't quite understand what the issue is: - kfree() releases memory allocated with kmalloc() - kasprintf() allocates memory using kmalloc_track_caller() Technically, I see no difficulty in freeing the newly allocated memory. In case of memory allocation issues in kasprintf, we would just get NULL, which kfree should also handle properly. Considering that we don't need soc_dev_attr anymore, we don't need to worry about the contents of .revision and .soc_id. I see that kfree_const has crept in by accident, which is essentially needed here only if we replace kasprintf with kasprintf_const for .soc_id, but it does not introduce any erroneous behavior. > >> + return PTR_ERR(soc_dev); >> + } >> + >> + dev = soc_device_to_device(soc_dev); >> + platform_set_drvdata(pdev, soc_dev); >> + >> + dev_info(dev, "Amlogic Meson %s Revision %x:%x (%x:%x) Detected (SM)\n", >> + soc_dev_attr->soc_id, >> + socinfo.v1.major_id, >> + socinfo.v1.chip_rev, >> + socinfo.v1.pack_id, >> + (socinfo.v1.reserved<<4) + socinfo.v1.layout_ver); >> + >> + return PTR_ERR_OR_ZERO(dev); >> +} >> + >> + >> +static int meson_gx_socinfo_sm_remove(struct platform_device *pdev) >> +{ >> + struct soc_device *soc_dev = platform_get_drvdata(pdev); >> + >> + soc_device_unregister(soc_dev); > > If you free the memory in probe() error path, why you did not decide to > free it here as well? It is symmetrical, so this should make you wonder > - error path is wrong. This is something I can easily explain: In the case where we have successfully registered with soc_device_register, we clean up everything that was manually allocated and not used. In the case of unloading the driver, the cleanup should be handled by the soc_device_unregister command. Technically, it's not possible to insert memory release because until the command is called, the driver is active, and afterwards, there's no guarantee of the pointer's validity. Perhaps it would have been better if soc_device_register copied the entire soc_device_attribute structure and took care of memory allocation and release itself, then we could comfortably free any excess memory back in _probe. Are you currently recommending not to release memory within the if (IS_ERR(soc_dev)) section?
On 14/03/2024 13:22, Viacheslav wrote: >> + >>> + soc_dev_attr = devm_kzalloc(&pdev->dev, sizeof(*soc_dev_attr), >>> + GFP_KERNEL); >>> + if (!soc_dev_attr) >>> + return -ENOMEM; >>> + >>> + soc_dev_attr->serial_number = socinfo_get_chipid(&pdev->dev, fw, &socinfo); >>> + >>> + soc_dev_attr->family = "Amlogic Meson"; >>> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x:%x - %x:%x", >>> + socinfo.v1.major_id, >>> + socinfo.v1.chip_rev, >>> + socinfo.v1.pack_id, >>> + (socinfo.v1.reserved<<4) + socinfo.v1.layout_ver); >>> + soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%s (%s)", >>> + socinfo_v1_to_soc_id(socinfo), >>> + socinfo_v1_to_package_id(socinfo)); >>> + >>> + soc_dev = soc_device_register(soc_dev_attr); >>> + >>> + >>> + if (IS_ERR(soc_dev)) { >>> + kfree(soc_dev_attr->revision); >>> + kfree_const(soc_dev_attr->soc_id); >>> + kfree(soc_dev_attr); >> >> That's a double free. This was not tested. > > > Please, describe the problem. Test your code. What's the point of arguing over it if regular test would show this? > I don't quite understand what the issue is: > > - kfree() releases memory allocated with kmalloc() So point me where is kmalloc(). I don't see. I see only devm. > - kasprintf() allocates memory using kmalloc_track_caller() > > Technically, I see no difficulty in freeing the newly allocated memory. > In case of memory allocation issues in kasprintf, we would just get > NULL, which kfree should also handle properly. Considering that we don't > need soc_dev_attr anymore, we don't need to worry about the contents of > .revision and .soc_id. Please pay attention that my comment is under specific line. We do not discuss unrelated code. > > I see that kfree_const has crept in by accident, which is essentially > needed here only if we replace kasprintf with kasprintf_const for > .soc_id, but it does not introduce any erroneous behavior. > >> >>> + return PTR_ERR(soc_dev); >>> + } >>> + >>> + dev = soc_device_to_device(soc_dev); >>> + platform_set_drvdata(pdev, soc_dev); >>> + >>> + dev_info(dev, "Amlogic Meson %s Revision %x:%x (%x:%x) Detected (SM)\n", >>> + soc_dev_attr->soc_id, >>> + socinfo.v1.major_id, >>> + socinfo.v1.chip_rev, >>> + socinfo.v1.pack_id, >>> + (socinfo.v1.reserved<<4) + socinfo.v1.layout_ver); >>> + >>> + return PTR_ERR_OR_ZERO(dev); >>> +} >>> + >>> + >>> +static int meson_gx_socinfo_sm_remove(struct platform_device *pdev) >>> +{ >>> + struct soc_device *soc_dev = platform_get_drvdata(pdev); >>> + >>> + soc_device_unregister(soc_dev); >> >> If you free the memory in probe() error path, why you did not decide to >> free it here as well? It is symmetrical, so this should make you wonder >> - error path is wrong. > > This is something I can easily explain: > > In the case where we have successfully registered with > soc_device_register, we clean up everything that was manually allocated > and not used. > In the case of unloading the driver, the cleanup should be handled by > the soc_device_unregister command. > > Technically, it's not possible to insert memory release because until > the command is called, the driver is active, and afterwards, there's no > guarantee of the pointer's validity. Then you do not understand lifecycle of device. There is release here via devm. Exactly at after my comment, when } finishes. > Perhaps it would have been better if soc_device_register copied the > entire soc_device_attribute structure and took care of memory allocation > and release itself, then we could comfortably free any excess memory > back in _probe. > > Are you currently recommending not to release memory within the if > (IS_ERR(soc_dev)) section? You have double free which will be pointed out by testing. Yes, of course I recommend not to have double free, so not to release memory which is being released. Best regards, Krzysztof
14/03/2024 16.31, Krzysztof Kozlowski wrote: > On 14/03/2024 13:22, Viacheslav wrote: >>> + >>>> + soc_dev_attr = devm_kzalloc(&pdev->dev, sizeof(*soc_dev_attr), >>>> + GFP_KERNEL); >>>> + if (!soc_dev_attr) >>>> + return -ENOMEM; >>>> + >>>> + soc_dev_attr->serial_number = socinfo_get_chipid(&pdev->dev, fw, &socinfo); >>>> + >>>> + soc_dev_attr->family = "Amlogic Meson"; >>>> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x:%x - %x:%x", >>>> + socinfo.v1.major_id, >>>> + socinfo.v1.chip_rev, >>>> + socinfo.v1.pack_id, >>>> + (socinfo.v1.reserved<<4) + socinfo.v1.layout_ver); >>>> + soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%s (%s)", >>>> + socinfo_v1_to_soc_id(socinfo), >>>> + socinfo_v1_to_package_id(socinfo)); >>>> + >>>> + soc_dev = soc_device_register(soc_dev_attr); >>>> + >>>> + >>>> + if (IS_ERR(soc_dev)) { >>>> + kfree(soc_dev_attr->revision); >>>> + kfree_const(soc_dev_attr->soc_id); >>>> + kfree(soc_dev_attr); >>> >>> That's a double free. This was not tested. >> >> >> Please, describe the problem. > > Test your code. What's the point of arguing over it if regular test > would show this? > >> I don't quite understand what the issue is: >> >> - kfree() releases memory allocated with kmalloc() > > So point me where is kmalloc(). I don't see. I see only devm. I missed the point that devm_kzalloc is automatically freed. You are right. Thanks!
diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig index d08e398bdad4..82fc77ca3b4b 100644 --- a/drivers/soc/amlogic/Kconfig +++ b/drivers/soc/amlogic/Kconfig @@ -26,6 +26,16 @@ config MESON_GX_SOCINFO Say yes to support decoding of Amlogic Meson GX SoC family information about the type, package and version. +config MESON_GX_SOCINFO_SM + bool "Amlogic Meson GX SoC Information driver via Secure Monitor" + depends on (ARM64 && ARCH_MESON || COMPILE_TEST) && MESON_SM=y + default ARCH_MESON && MESON_SM + select SOC_BUS + help + Say yes to support decoding of Amlogic Meson GX SoC family + information about the type, package and version via secure + monitor call. + config MESON_MX_SOCINFO bool "Amlogic Meson MX SoC Information driver" depends on (ARM && ARCH_MESON) || COMPILE_TEST diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile index c25f835e6a26..45d9d6f5904c 100644 --- a/drivers/soc/amlogic/Makefile +++ b/drivers/soc/amlogic/Makefile @@ -2,4 +2,5 @@ obj-$(CONFIG_MESON_CANVAS) += meson-canvas.o obj-$(CONFIG_MESON_CLK_MEASURE) += meson-clk-measure.o obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o +obj-$(CONFIG_MESON_GX_SOCINFO_SM) += meson-gx-socinfo-sm.o obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o diff --git a/drivers/soc/amlogic/meson-gx-socinfo-sm.c b/drivers/soc/amlogic/meson-gx-socinfo-sm.c new file mode 100644 index 000000000000..e30e1d2feb61 --- /dev/null +++ b/drivers/soc/amlogic/meson-gx-socinfo-sm.c @@ -0,0 +1,192 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2017 BayLibre, SAS + * Copyright (c) 2024 JetHome + * Author: Neil Armstrong <neil.armstrong@linaro.org> + * Author: Viacheslav Bocharov <adeep@lexina.in> + * + */ + +#include <linux/io.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/of_platform.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/sys_soc.h> +#include <linux/regmap.h> +#include <linux/mfd/syscon.h> + +#include <linux/firmware/meson/meson_sm.h> + +#include "meson-gx-socinfo-internal.h" + +static char *socinfo_get_chipid(struct device *dev, struct meson_sm_firmware *fw, + union meson_cpu_id *socinfo) +{ + char *buf; + struct meson_sm_chip_id *id_buf; + int ret; + + id_buf = devm_kzalloc(dev, sizeof(struct meson_sm_chip_id)+1, GFP_KERNEL); + if (!id_buf) + return NULL; + + ret = meson_sm_call_read(fw, id_buf, sizeof(struct meson_sm_chip_id), SM_GET_CHIP_ID, + 2, 0, 0, 0, 0); + if (ret < 0) { + kfree(id_buf); + return NULL; + } + dev_info(dev, "got sm version call %i\n", id_buf->version); + + if (id_buf->version != 2) { + + u8 tmp; + /** + * Legacy 12-byte chip ID read out, transform data + * to expected order format + */ + memmove((void *)&id_buf->serial, (void *)&id_buf->cpu_id, 12); + for (int i = 0; i < 6; i++) { + tmp = id_buf->serial[i]; + id_buf->serial[i] = id_buf->serial[11 - i]; + id_buf->serial[11 - i] = tmp; + } + id_buf->cpu_id.v2.major_id = socinfo->v1.major_id; + id_buf->cpu_id.v2.pack_id = socinfo->v1.pack_id; + id_buf->cpu_id.v2.chip_rev = socinfo->v1.chip_rev; + id_buf->cpu_id.v2.reserved = socinfo->v1.reserved; + id_buf->cpu_id.v2.layout_ver = socinfo->v1.layout_ver; + } else { + /** + * rewrite socinfo from regmap with value from secure monitor call + */ + socinfo->v1.major_id = id_buf->cpu_id.v2.major_id; + socinfo->v1.pack_id = id_buf->cpu_id.v2.pack_id; + socinfo->v1.chip_rev = id_buf->cpu_id.v2.chip_rev; + socinfo->v1.reserved = id_buf->cpu_id.v2.reserved; + socinfo->v1.layout_ver = id_buf->cpu_id.v2.layout_ver; + } + + buf = kasprintf(GFP_KERNEL, "%4phN%12phN", &(id_buf->cpu_id), &(id_buf->serial)); + + kfree(id_buf); + + return buf; +} + +static int meson_gx_socinfo_sm_probe(struct platform_device *pdev) +{ + struct soc_device_attribute *soc_dev_attr; + struct soc_device *soc_dev; + struct device_node *sm_np; + struct meson_sm_firmware *fw; + struct regmap *regmap; + union meson_cpu_id socinfo; + struct device *dev; + int ret; + + /* check if chip-id is available */ + if (!of_property_read_bool(pdev->dev.of_node, "amlogic,has-chip-id")) + return -ENODEV; + + /* node should be a syscon */ + regmap = syscon_node_to_regmap(pdev->dev.of_node); + if (IS_ERR(regmap)) { + dev_err(&pdev->dev, "failed to get regmap\n"); + return -ENODEV; + } + + sm_np = of_parse_phandle(pdev->dev.of_node, "secure-monitor", 0); + if (!sm_np) { + dev_err(&pdev->dev, "no secure-monitor node found\n"); + return -ENODEV; + } + + fw = meson_sm_get(sm_np); + of_node_put(sm_np); + if (!fw) { + dev_info(&pdev->dev, "secure-monitor device not ready, probe later\n"); + return -EPROBE_DEFER; + } + + ret = regmap_read(regmap, AO_SEC_SOCINFO_OFFSET, &socinfo.raw); + if (ret < 0) + return ret; + + if (!socinfo.raw) { + dev_err(&pdev->dev, "invalid regmap chipid value\n"); + return -EINVAL; + } + + soc_dev_attr = devm_kzalloc(&pdev->dev, sizeof(*soc_dev_attr), + GFP_KERNEL); + if (!soc_dev_attr) + return -ENOMEM; + + soc_dev_attr->serial_number = socinfo_get_chipid(&pdev->dev, fw, &socinfo); + + soc_dev_attr->family = "Amlogic Meson"; + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x:%x - %x:%x", + socinfo.v1.major_id, + socinfo.v1.chip_rev, + socinfo.v1.pack_id, + (socinfo.v1.reserved<<4) + socinfo.v1.layout_ver); + soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%s (%s)", + socinfo_v1_to_soc_id(socinfo), + socinfo_v1_to_package_id(socinfo)); + + soc_dev = soc_device_register(soc_dev_attr); + + + if (IS_ERR(soc_dev)) { + kfree(soc_dev_attr->revision); + kfree_const(soc_dev_attr->soc_id); + kfree(soc_dev_attr); + return PTR_ERR(soc_dev); + } + + dev = soc_device_to_device(soc_dev); + platform_set_drvdata(pdev, soc_dev); + + dev_info(dev, "Amlogic Meson %s Revision %x:%x (%x:%x) Detected (SM)\n", + soc_dev_attr->soc_id, + socinfo.v1.major_id, + socinfo.v1.chip_rev, + socinfo.v1.pack_id, + (socinfo.v1.reserved<<4) + socinfo.v1.layout_ver); + + return PTR_ERR_OR_ZERO(dev); +} + + +static int meson_gx_socinfo_sm_remove(struct platform_device *pdev) +{ + struct soc_device *soc_dev = platform_get_drvdata(pdev); + + soc_device_unregister(soc_dev); + return 0; +} + +static const struct of_device_id meson_gx_socinfo_match[] = { + { .compatible = "amlogic,meson-gx-ao-secure", }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(of, meson_gx_socinfo_match); + +static struct platform_driver meson_gx_socinfo_driver = { + .probe = meson_gx_socinfo_sm_probe, + .remove = meson_gx_socinfo_sm_remove, + .driver = { + .name = "meson-gx-socinfo-sm", + .of_match_table = meson_gx_socinfo_match, + }, +}; + + +module_platform_driver(meson_gx_socinfo_driver); + +MODULE_AUTHOR("Viacheslav Bocharov <adeep@lexina.in>"); +MODULE_DESCRIPTION("Amlogic Meson GX SOC SM driver"); +MODULE_LICENSE("GPL");
Amlogic SoCs have a SoC information secure-monitor call for SoC type, package type, revision information and chipid. This patchs adds support for secure-monitor call decoding and exposing with the SoC bus infrastructure in addition to the previous SoC Information driver. Signed-off-by: Viacheslav Bocharov <adeep@lexina.in> --- drivers/soc/amlogic/Kconfig | 10 ++ drivers/soc/amlogic/Makefile | 1 + drivers/soc/amlogic/meson-gx-socinfo-sm.c | 192 ++++++++++++++++++++++ 3 files changed, 203 insertions(+) create mode 100644 drivers/soc/amlogic/meson-gx-socinfo-sm.c