| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1437649445.30329.90.camel@mtksdaap41>
Date: Thu, 23 Jul 2015 19:04:05 +0800
From: Henry Chen <HenryC.Chen@...iatek.com>
To: Javier Martinez Canillas <javier@...hile0.org>
CC: Mark Brown <broonie@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Liam Girdwood <lgirdwood@...il.com>,
Linux Kernel <linux-kernel@...r.kernel.org>,
<linux-mediatek@...ts.infradead.org>,
"Sascha Hauer" <kernel@...gutronix.de>,
Matthias Brugger <matthias.bgg@...il.com>,
<eddie.huang@...iatek.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3 2/2] regulator: mt6311: Add support for mt6311
regulator
On Thu, 2015-07-23 at 11:07 +0200, Javier Martinez Canillas wrote:
> > +
> > + if (ret < 0)
> > + dev_err(&i2c->dev, "Failed to initialize regulator: %d\n", ret);
> > +
>
> I don't think this is necessary, you are already adding logs for all
> the error conditions.
Yes, I will remove it.
>
> > + return ret;
> > +}
> > +
> > +static const struct i2c_device_id mt6311_i2c_id[] = {
> > + {"mt6311", 0},
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(i2c, mt6311_i2c_id);
> > +
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id mt6311_dt_ids[] = {
> > + { .compatible = "mediatek,mt6311-regulator",
> > + .data = &mt6311_i2c_id[0] },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, mt6311_dt_ids);
> > +#endif
> > +
> > +static struct i2c_driver mt6311_regulator_driver = {
> > + .driver = {
> > + .name = "mt6311",
> > + .owner = THIS_MODULE,
> > + .of_match_table = of_match_ptr(mt6311_dt_ids),
>
> of_match_ptr() is NULL when CONFIG_OF is not defined so you don't need
> the additional #ifdef
> CONFIG_OF around mt6311_dt_ids.
If CONFIG_OF is not defined and without #ifdef CONFIG_OF around
mt6311_dt_ids, it will allocate a struct mt6311_dt_ids but no one used
on here, right?
Henry
> > + },
> > + .probe = mt6311_i2c_probe,
> > + .id_table = mt6311_i2c_id,
> > +};
> > +
> > +module_i2c_driver(mt6311_regulator_driver);
> > +
> > +MODULE_AUTHOR("Henry Chen <henryc.chen@...iatek.com>");
> > +MODULE_DESCRIPTION("Regulator device driver for Mediatek MT6311");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/regulator/mt6311-regulator.h b/drivers/regulator/mt6311-regulator.h
> > new file mode 100644
> > index 0000000..5218db4
> > --- /dev/null
> > +++ b/drivers/regulator/mt6311-regulator.h
> > @@ -0,0 +1,65 @@
> > +/*
> > + * Copyright (c) 2015 MediaTek Inc.
> > + * Author: Henry Chen <henryc.chen@...iatek.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#ifndef __MT6311_REGULATOR_H__
> > +#define __MT6311_REGULATOR_H__
> > +
> > +#define MT6311_SWCID 0x01
> > +
> > +#define MT6311_TOP_INT_CON 0x18
> > +#define MT6311_TOP_INT_MON 0x19
> > +
> > +#define MT6311_VDVFS11_CON0 0x87
> > +#define MT6311_VDVFS11_CON7 0x88
> > +#define MT6311_VDVFS11_CON8 0x89
> > +#define MT6311_VDVFS11_CON9 0x8A
> > +#define MT6311_VDVFS11_CON10 0x8B
> > +#define MT6311_VDVFS11_CON11 0x8C
> > +#define MT6311_VDVFS11_CON12 0x8D
> > +#define MT6311_VDVFS11_CON13 0x8E
> > +#define MT6311_VDVFS11_CON14 0x8F
> > +#define MT6311_VDVFS11_CON15 0x90
> > +#define MT6311_VDVFS11_CON16 0x91
> > +#define MT6311_VDVFS11_CON17 0x92
> > +#define MT6311_VDVFS11_CON18 0x93
> > +#define MT6311_VDVFS11_CON19 0x94
> > +
> > +#define MT6311_LDO_CON0 0xCC
> > +#define MT6311_LDO_OCFB0 0xCD
> > +#define MT6311_LDO_CON2 0xCE
> > +#define MT6311_LDO_CON3 0xCF
> > +#define MT6311_LDO_CON4 0xD0
> > +#define MT6311_FQMTR_CON0 0xD1
> > +#define MT6311_FQMTR_CON1 0xD2
> > +#define MT6311_FQMTR_CON2 0xD3
> > +#define MT6311_FQMTR_CON3 0xD4
> > +#define MT6311_FQMTR_CON4 0xD5
> > +
> > +#define MT6311_PMIC_RG_INT_POL_MASK 0x1
> > +#define MT6311_PMIC_RG_INT_EN_MASK 0x2
> > +#define MT6311_PMIC_RG_BUCK_OC_INT_STATUS_MASK 0x10
> > +
> > +#define MT6311_PMIC_VDVFS11_EN_CTRL_MASK 0x1
> > +#define MT6311_PMIC_VDVFS11_VOSEL_CTRL_MASK 0x2
> > +#define MT6311_PMIC_VDVFS11_EN_SEL_MASK 0x3
> > +#define MT6311_PMIC_VDVFS11_VOSEL_SEL_MASK 0xc
> > +#define MT6311_PMIC_VDVFS11_EN_MASK 0x1
> > +#define MT6311_PMIC_VDVFS11_VOSEL_MASK 0x7F
> > +#define MT6311_PMIC_VDVFS11_VOSEL_ON_MASK 0x7F
> > +#define MT6311_PMIC_VDVFS11_VOSEL_SLEEP_MASK 0x7F
> > +#define MT6311_PMIC_NI_VDVFS11_VOSEL_MASK 0x7F
> > +
> > +#define MT6311_PMIC_RG_VBIASN_EN_MASK 0x1
> > +
> > +#endif
> > diff --git a/include/linux/regulator/mt6311.h b/include/linux/regulator/mt6311.h
> > new file mode 100644
> > index 0000000..8473259
> > --- /dev/null
> > +++ b/include/linux/regulator/mt6311.h
> > @@ -0,0 +1,29 @@
> > +/*
> > + * Copyright (c) 2015 MediaTek Inc.
> > + * Author: Henry Chen <henryc.chen@...iatek.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#ifndef __LINUX_REGULATOR_MT6311_H
> > +#define __LINUX_REGULATOR_MT6311_H
> > +
> > +#define MT6311_MAX_REGULATORS 2
> > +
> > +enum {
> > + MT6311_ID_VDVFS = 0,
> > + MT6311_ID_VBIASN,
> > +};
> > +
> > +#define MT6311_E1_CID_CODE 0x10
> > +#define MT6311_E2_CID_CODE 0x20
> > +#define MT6311_E3_CID_CODE 0x30
> > +
> > +#endif /* __LINUX_REGULATOR_MT6311_H */
> > --
>
> Besides those comments, it looks good to me now.
>
> Reviewed-by: Javier Martinez Canillas <javier@....samsung.com>
>
> Best regards,
> Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists