Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1356133
| From | Joe Perches <joe@perches.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v2] bsp: add SoC driver to brcmstb |
| Date | 2016-03-11 20:00 +0100 |
| Message-ID | <rbAs2-1Yo-11@gated-at.bofh.it> (permalink) |
| References | <rbAin-1UI-25@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Fri, 2016-03-11 at 10:39 -0800, Justin Chen wrote:
> From: Justin Chen <justin.chen@broadcom.com>
>
> Value of soc_dev_attributes:
> family = chip family id
> soc_id = product id
> revision = product revision
[]
> diff --git a/drivers/soc/brcmstb/common.c b/drivers/soc/brcmstb/common.c
[]
> +static int __init brcmstb_soc_device_init(void)
> +{
> + struct soc_device_attribute *soc_dev_attr;
> + struct soc_device *soc_dev;
> + struct device_node *sun_top_ctrl;
> + void __iomem *sun_top_ctrl_base;
> +
> + sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
> + if (!sun_top_ctrl)
> + return -ENODEV;
> +
> + sun_top_ctrl_base = of_iomap(sun_top_ctrl, 0);
> + if (!sun_top_ctrl_base)
> + return -ENODEV;
> +
> + family_id = readl(sun_top_ctrl_base);
> + product_id = readl(sun_top_ctrl_base + 0x4);
> +
> + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> + if (!soc_dev_attr)
> + return -ENOMEM;
> +
> + soc_dev_attr->family = kasprintf(GFP_KERNEL, "%x",
> + family_id >> 28 ?
> + family_id >> 16 : family_id >> 8);
> + soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%x",
> + product_id >> 28 ?
> + product_id >> 16 : product_id >> 8);
> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%c%d",
> + (char)(((product_id & 0xff) >> 4) + 65),
> + product_id & 0xf);
This might be easier to read as:
soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%c%d"
((product_id & 0xf0) >> 4) + 'A',
product_id & 0xf);
> +
> + soc_dev = soc_device_register(soc_dev_attr);
> + if (IS_ERR(soc_dev)) {
> + kfree(soc_dev_attr->family);
> + kfree(soc_dev_attr->soc_id);
> + kfree(soc_dev_attr->revision);
> + kfree(soc_dev_attr);
> + return -1;
Mixing GENERIC_ERRNO types and fixed values seems odd.
Is -EPERM the correct return here? Maybe -ENODEV?
> + }
> +
> + return 0;
> +}
> +arch_initcall(brcmstb_soc_device_init);
Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread
[PATCH v2] bsp: add SoC driver to brcmstb Justin Chen <justinpopo6@gmail.com> - 2016-03-11 19:50 +0100 Re: [PATCH v2] bsp: add SoC driver to brcmstb Joe Perches <joe@perches.com> - 2016-03-11 20:00 +0100
csiph-web