| Message ID | 20200323121537.22697-8-michael.kao@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Mediatek thermal dirver and dtsi | expand |
On Mon, 2020-03-23 at 20:15 +0800, Michael Kao wrote: > From: "michael.kao" <michael.kao@mediatek.com> > > The driver of thermal and svs will use the > same register for the project which should select > bank before reading sensor value. > > Signed-off-by: Michael Kao <michael.kao@mediatek.com> > --- > drivers/thermal/mtk_thermal.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c > index 9eaca432920e..594ad4f0f8cd 100644 > --- a/drivers/thermal/mtk_thermal.c > +++ b/drivers/thermal/mtk_thermal.c > @@ -22,6 +22,7 @@ > #include <linux/thermal.h> > #include <linux/reset.h> > #include <linux/types.h> > +#include <linux/power/mtk_svs.h> > > /* AUXADC Registers */ > #define AUXADC_CON1_SET_V 0x008 > @@ -262,7 +263,7 @@ struct mtk_thermal { > struct clk *clk_peri_therm; > struct clk *clk_auxadc; > /* lock: for getting and putting banks */ > - struct mutex lock; > + unsigned long flags; > > /* Calibration values */ > s32 adc_ge; > @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank) > u32 val; > > if (mt->conf->need_switch_bank) { > - mutex_lock(&mt->lock); > + mt->flags = claim_mtk_svs_lock(); > > val = readl(mt->thermal_base + PTPCORESEL); > val &= ~0xf; > @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank) > struct mtk_thermal *mt = bank->mt; > > if (mt->conf->need_switch_bank) > - mutex_unlock(&mt->lock); > + release_mtk_svs_lock(mt->flags); > } > > /** > @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev) > if (ret) > return ret; > > - mutex_init(&mt->lock); > - > mt->dev = &pdev->dev; > > auxadc = of_parse_phandle(np, "mediatek,auxadc", 0);
On Mon, 2020-03-23 at 20:15 +0800, Michael Kao wrote: > From: "michael.kao" <michael.kao@mediatek.com> > > The driver of thermal and svs will use the > same register for the project which should select > bank before reading sensor value. > > Signed-off-by: Michael Kao <michael.kao@mediatek.com> > --- > drivers/thermal/mtk_thermal.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c > index 9eaca432920e..594ad4f0f8cd 100644 > --- a/drivers/thermal/mtk_thermal.c > +++ b/drivers/thermal/mtk_thermal.c > @@ -22,6 +22,7 @@ > #include <linux/thermal.h> > #include <linux/reset.h> > #include <linux/types.h> > +#include <linux/power/mtk_svs.h> > > /* AUXADC Registers */ > #define AUXADC_CON1_SET_V 0x008 > @@ -262,7 +263,7 @@ struct mtk_thermal { > struct clk *clk_peri_therm; > struct clk *clk_auxadc; > /* lock: for getting and putting banks */ > - struct mutex lock; > + unsigned long flags; > > /* Calibration values */ > s32 adc_ge; > @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank) > u32 val; > > if (mt->conf->need_switch_bank) { > - mutex_lock(&mt->lock); > + mt->flags = claim_mtk_svs_lock(); > > val = readl(mt->thermal_base + PTPCORESEL); > val &= ~0xf; > @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank) > struct mtk_thermal *mt = bank->mt; > > if (mt->conf->need_switch_bank) > - mutex_unlock(&mt->lock); > + release_mtk_svs_lock(mt->flags); > } > > /** > @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev) > if (ret) > return ret; > > - mutex_init(&mt->lock); > - > mt->dev = &pdev->dev; > > auxadc = of_parse_phandle(np, "mediatek,auxadc", 0); Hi Matthias, Kindly ping.
On Mon, 2020-03-23 at 20:15 +0800, Michael Kao wrote: > From: "michael.kao" <michael.kao@mediatek.com> > > The driver of thermal and svs will use the > same register for the project which should select > bank before reading sensor value. > > Signed-off-by: Michael Kao <michael.kao@mediatek.com> > --- > drivers/thermal/mtk_thermal.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c > index 9eaca432920e..594ad4f0f8cd 100644 > --- a/drivers/thermal/mtk_thermal.c > +++ b/drivers/thermal/mtk_thermal.c > @@ -22,6 +22,7 @@ > #include <linux/thermal.h> > #include <linux/reset.h> > #include <linux/types.h> > +#include <linux/power/mtk_svs.h> > > /* AUXADC Registers */ > #define AUXADC_CON1_SET_V 0x008 > @@ -262,7 +263,7 @@ struct mtk_thermal { > struct clk *clk_peri_therm; > struct clk *clk_auxadc; > /* lock: for getting and putting banks */ > - struct mutex lock; > + unsigned long flags; > > /* Calibration values */ > s32 adc_ge; > @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank) > u32 val; > > if (mt->conf->need_switch_bank) { > - mutex_lock(&mt->lock); > + mt->flags = claim_mtk_svs_lock(); > > val = readl(mt->thermal_base + PTPCORESEL); > val &= ~0xf; > @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank) > struct mtk_thermal *mt = bank->mt; > > if (mt->conf->need_switch_bank) > - mutex_unlock(&mt->lock); > + release_mtk_svs_lock(mt->flags); > } > > /** > @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev) > if (ret) > return ret; > > - mutex_init(&mt->lock); > - > mt->dev = &pdev->dev; > > auxadc = of_parse_phandle(np, "mediatek,auxadc", 0);
On 14/05/2020 08:55, Michael Kao wrote: > On Mon, 2020-03-23 at 20:15 +0800, Michael Kao wrote: >> From: "michael.kao" <michael.kao@mediatek.com> >> >> The driver of thermal and svs will use the >> same register for the project which should select >> bank before reading sensor value. >> >> Signed-off-by: Michael Kao <michael.kao@mediatek.com> >> --- >> drivers/thermal/mtk_thermal.c | 9 ++++----- >> 1 file changed, 4 insertions(+), 5 deletions(-) >> [ ... ] > Hi Matthias, Those patches fall under the thermal framework umbrella. Thanks -- Daniel
On 23/03/2020 13:15, Michael Kao wrote: > From: "michael.kao" <michael.kao@mediatek.com> > > The driver of thermal and svs will use the > same register for the project which should select > bank before reading sensor value. Here there is a design problem AFAICT. The sensor should not be using external locks. > Signed-off-by: Michael Kao <michael.kao@mediatek.com> > --- > drivers/thermal/mtk_thermal.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c > index 9eaca432920e..594ad4f0f8cd 100644 > --- a/drivers/thermal/mtk_thermal.c > +++ b/drivers/thermal/mtk_thermal.c > @@ -22,6 +22,7 @@ > #include <linux/thermal.h> > #include <linux/reset.h> > #include <linux/types.h> > +#include <linux/power/mtk_svs.h> > > /* AUXADC Registers */ > #define AUXADC_CON1_SET_V 0x008 > @@ -262,7 +263,7 @@ struct mtk_thermal { > struct clk *clk_peri_therm; > struct clk *clk_auxadc; > /* lock: for getting and putting banks */ > - struct mutex lock; > + unsigned long flags; > > /* Calibration values */ > s32 adc_ge; > @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank) > u32 val; > > if (mt->conf->need_switch_bank) { > - mutex_lock(&mt->lock); > + mt->flags = claim_mtk_svs_lock(); > > val = readl(mt->thermal_base + PTPCORESEL); > val &= ~0xf; > @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank) > struct mtk_thermal *mt = bank->mt; > > if (mt->conf->need_switch_bank) > - mutex_unlock(&mt->lock); > + release_mtk_svs_lock(mt->flags); > } > > /** > @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev) > if (ret) > return ret; > > - mutex_init(&mt->lock); > - > mt->dev = &pdev->dev; > > auxadc = of_parse_phandle(np, "mediatek,auxadc", 0); >
On Fri, 2020-05-22 at 17:36 +0200, Daniel Lezcano wrote: > On 23/03/2020 13:15, Michael Kao wrote: > > From: "michael.kao" <michael.kao@mediatek.com> > > > > The driver of thermal and svs will use the > > same register for the project which should select > > bank before reading sensor value. > > Here there is a design problem AFAICT. The sensor should not be using > external locks. > The PTPCORESEL is a common register used by svs and thermal. The thermal need to ensure PTPCORESEL register will not be changed by svs when thermal switch bank to read raw data of temperature. So we use svs_lock to make sure there is no conflict between the two drivers. > > > Signed-off-by: Michael Kao <michael.kao@mediatek.com> > > --- > > drivers/thermal/mtk_thermal.c | 9 ++++----- > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c > > index 9eaca432920e..594ad4f0f8cd 100644 > > --- a/drivers/thermal/mtk_thermal.c > > +++ b/drivers/thermal/mtk_thermal.c > > @@ -22,6 +22,7 @@ > > #include <linux/thermal.h> > > #include <linux/reset.h> > > #include <linux/types.h> > > +#include <linux/power/mtk_svs.h> > > > > /* AUXADC Registers */ > > #define AUXADC_CON1_SET_V 0x008 > > @@ -262,7 +263,7 @@ struct mtk_thermal { > > struct clk *clk_peri_therm; > > struct clk *clk_auxadc; > > /* lock: for getting and putting banks */ > > - struct mutex lock; > > + unsigned long flags; > > > > /* Calibration values */ > > s32 adc_ge; > > @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank) > > u32 val; > > > > if (mt->conf->need_switch_bank) { > > - mutex_lock(&mt->lock); > > + mt->flags = claim_mtk_svs_lock(); > > > > val = readl(mt->thermal_base + PTPCORESEL); > > val &= ~0xf; > > @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank) > > struct mtk_thermal *mt = bank->mt; > > > > if (mt->conf->need_switch_bank) > > - mutex_unlock(&mt->lock); > > + release_mtk_svs_lock(mt->flags); > > } > > > > /** > > @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev) > > if (ret) > > return ret; > > > > - mutex_init(&mt->lock); > > - > > mt->dev = &pdev->dev; > > > > auxadc = of_parse_phandle(np, "mediatek,auxadc", 0); > > > >
On Fri, 2020-06-05 at 11:50 +0800, Michael Kao wrote: > On Fri, 2020-05-22 at 17:36 +0200, Daniel Lezcano wrote: > > On 23/03/2020 13:15, Michael Kao wrote: > > > From: "michael.kao" <michael.kao@mediatek.com> > > > > > > The driver of thermal and svs will use the > > > same register for the project which should select > > > bank before reading sensor value. > > > > Here there is a design problem AFAICT. The sensor should not be using > > external locks. > > > The PTPCORESEL is a common register used by svs and thermal. > The thermal need to ensure PTPCORESEL register will not be changed by > svs when thermal switch bank to read raw data of temperature. > So we use svs_lock to make sure there is no conflict between the two > drivers. > > > > > Signed-off-by: Michael Kao <michael.kao@mediatek.com> > > > --- > > > drivers/thermal/mtk_thermal.c | 9 ++++----- > > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c > > > index 9eaca432920e..594ad4f0f8cd 100644 > > > --- a/drivers/thermal/mtk_thermal.c > > > +++ b/drivers/thermal/mtk_thermal.c > > > @@ -22,6 +22,7 @@ > > > #include <linux/thermal.h> > > > #include <linux/reset.h> > > > #include <linux/types.h> > > > +#include <linux/power/mtk_svs.h> > > > > > > /* AUXADC Registers */ > > > #define AUXADC_CON1_SET_V 0x008 > > > @@ -262,7 +263,7 @@ struct mtk_thermal { > > > struct clk *clk_peri_therm; > > > struct clk *clk_auxadc; > > > /* lock: for getting and putting banks */ > > > - struct mutex lock; > > > + unsigned long flags; > > > > > > /* Calibration values */ > > > s32 adc_ge; > > > @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank) > > > u32 val; > > > > > > if (mt->conf->need_switch_bank) { > > > - mutex_lock(&mt->lock); > > > + mt->flags = claim_mtk_svs_lock(); > > > > > > val = readl(mt->thermal_base + PTPCORESEL); > > > val &= ~0xf; > > > @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank) > > > struct mtk_thermal *mt = bank->mt; > > > > > > if (mt->conf->need_switch_bank) > > > - mutex_unlock(&mt->lock); > > > + release_mtk_svs_lock(mt->flags); > > > } > > > > > > /** > > > @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev) > > > if (ret) > > > return ret; > > > > > > - mutex_init(&mt->lock); > > > - > > > mt->dev = &pdev->dev; > > > > > > auxadc = of_parse_phandle(np, "mediatek,auxadc", 0); > > > > > > > > Hi Daniel, Gently ping. > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On 07/07/2020 04:57, Michael Kao wrote:
[ ... ]
> Gently ping.
Michael,
it is impossible to have this series merged if there is a so big delay
between the comments / questions and the answers (or no answers at all).
On 05/06/2020 05:50, Michael Kao wrote: > On Fri, 2020-05-22 at 17:36 +0200, Daniel Lezcano wrote: >> On 23/03/2020 13:15, Michael Kao wrote: >>> From: "michael.kao" <michael.kao@mediatek.com> >>> >>> The driver of thermal and svs will use the >>> same register for the project which should select >>> bank before reading sensor value. >> >> Here there is a design problem AFAICT. The sensor should not be using >> external locks. >> > The PTPCORESEL is a common register used by svs and thermal. > The thermal need to ensure PTPCORESEL register will not be changed by > svs when thermal switch bank to read raw data of temperature. > So we use svs_lock to make sure there is no conflict between the two > drivers. Why not use regmap ? >>> Signed-off-by: Michael Kao <michael.kao@mediatek.com> >>> --- >>> drivers/thermal/mtk_thermal.c | 9 ++++----- >>> 1 file changed, 4 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c >>> index 9eaca432920e..594ad4f0f8cd 100644 >>> --- a/drivers/thermal/mtk_thermal.c >>> +++ b/drivers/thermal/mtk_thermal.c >>> @@ -22,6 +22,7 @@ >>> #include <linux/thermal.h> >>> #include <linux/reset.h> >>> #include <linux/types.h> >>> +#include <linux/power/mtk_svs.h> >>> >>> /* AUXADC Registers */ >>> #define AUXADC_CON1_SET_V 0x008 >>> @@ -262,7 +263,7 @@ struct mtk_thermal { >>> struct clk *clk_peri_therm; >>> struct clk *clk_auxadc; >>> /* lock: for getting and putting banks */ >>> - struct mutex lock; >>> + unsigned long flags; >>> >>> /* Calibration values */ >>> s32 adc_ge; >>> @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank) >>> u32 val; >>> >>> if (mt->conf->need_switch_bank) { >>> - mutex_lock(&mt->lock); >>> + mt->flags = claim_mtk_svs_lock(); >>> >>> val = readl(mt->thermal_base + PTPCORESEL); >>> val &= ~0xf; >>> @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank) >>> struct mtk_thermal *mt = bank->mt; >>> >>> if (mt->conf->need_switch_bank) >>> - mutex_unlock(&mt->lock); >>> + release_mtk_svs_lock(mt->flags); >>> } >>> >>> /** >>> @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev) >>> if (ret) >>> return ret; >>> >>> - mutex_init(&mt->lock); >>> - >>> mt->dev = &pdev->dev; >>> >>> auxadc = of_parse_phandle(np, "mediatek,auxadc", 0); >>> >> >> >
On Tue, 2020-07-07 at 10:13 +0200, Daniel Lezcano wrote: > On 05/06/2020 05:50, Michael Kao wrote: > > On Fri, 2020-05-22 at 17:36 +0200, Daniel Lezcano wrote: > >> On 23/03/2020 13:15, Michael Kao wrote: > >>> From: "michael.kao" <michael.kao@mediatek.com> > >>> > >>> The driver of thermal and svs will use the > >>> same register for the project which should select > >>> bank before reading sensor value. > >> > >> Here there is a design problem AFAICT. The sensor should not be using > >> external locks. > >> > > The PTPCORESEL is a common register used by svs and thermal. > > The thermal need to ensure PTPCORESEL register will not be changed by > > svs when thermal switch bank to read raw data of temperature. > > So we use svs_lock to make sure there is no conflict between the two > > drivers. > > Why not use regmap ? > Hi Daniel, We are not sure the regmap can solve the problem. The regmap can lock CORESELECT. But it can not protect the other sensors registers which thermal controller start to read temperature need to access. After internal disscussion, we don't have this kind of experience to use regmap to protect some peice of code. We will do some research of this kind of usage. Thanks for your suggestion. > >>> Signed-off-by: Michael Kao <michael.kao@mediatek.com> > >>> --- > >>> drivers/thermal/mtk_thermal.c | 9 ++++----- > >>> 1 file changed, 4 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c > >>> index 9eaca432920e..594ad4f0f8cd 100644 > >>> --- a/drivers/thermal/mtk_thermal.c > >>> +++ b/drivers/thermal/mtk_thermal.c > >>> @@ -22,6 +22,7 @@ > >>> #include <linux/thermal.h> > >>> #include <linux/reset.h> > >>> #include <linux/types.h> > >>> +#include <linux/power/mtk_svs.h> > >>> > >>> /* AUXADC Registers */ > >>> #define AUXADC_CON1_SET_V 0x008 > >>> @@ -262,7 +263,7 @@ struct mtk_thermal { > >>> struct clk *clk_peri_therm; > >>> struct clk *clk_auxadc; > >>> /* lock: for getting and putting banks */ > >>> - struct mutex lock; > >>> + unsigned long flags; > >>> > >>> /* Calibration values */ > >>> s32 adc_ge; > >>> @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank) > >>> u32 val; > >>> > >>> if (mt->conf->need_switch_bank) { > >>> - mutex_lock(&mt->lock); > >>> + mt->flags = claim_mtk_svs_lock(); > >>> > >>> val = readl(mt->thermal_base + PTPCORESEL); > >>> val &= ~0xf; > >>> @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank) > >>> struct mtk_thermal *mt = bank->mt; > >>> > >>> if (mt->conf->need_switch_bank) > >>> - mutex_unlock(&mt->lock); > >>> + release_mtk_svs_lock(mt->flags); > >>> } > >>> > >>> /** > >>> @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev) > >>> if (ret) > >>> return ret; > >>> > >>> - mutex_init(&mt->lock); > >>> - > >>> mt->dev = &pdev->dev; > >>> > >>> auxadc = of_parse_phandle(np, "mediatek,auxadc", 0); > >>> > >> > >> > > > >
diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c index 9eaca432920e..594ad4f0f8cd 100644 --- a/drivers/thermal/mtk_thermal.c +++ b/drivers/thermal/mtk_thermal.c @@ -22,6 +22,7 @@ #include <linux/thermal.h> #include <linux/reset.h> #include <linux/types.h> +#include <linux/power/mtk_svs.h> /* AUXADC Registers */ #define AUXADC_CON1_SET_V 0x008 @@ -262,7 +263,7 @@ struct mtk_thermal { struct clk *clk_peri_therm; struct clk *clk_auxadc; /* lock: for getting and putting banks */ - struct mutex lock; + unsigned long flags; /* Calibration values */ s32 adc_ge; @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank) u32 val; if (mt->conf->need_switch_bank) { - mutex_lock(&mt->lock); + mt->flags = claim_mtk_svs_lock(); val = readl(mt->thermal_base + PTPCORESEL); val &= ~0xf; @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank) struct mtk_thermal *mt = bank->mt; if (mt->conf->need_switch_bank) - mutex_unlock(&mt->lock); + release_mtk_svs_lock(mt->flags); } /** @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev) if (ret) return ret; - mutex_init(&mt->lock); - mt->dev = &pdev->dev; auxadc = of_parse_phandle(np, "mediatek,auxadc", 0);