2016-11-07 10:48:53

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH RESEND] phy: exynos-mipi-video: simplify check for coupled phy status

There is no need to access regmap of coupled phy to check its state - such
information is already in the phy device itself, so use it directly. This
let us to avoid possible access to registers of the device in the disabled
power domain if the coupled phy is already disabled.

Signed-off-by: Marek Szyprowski <[email protected]>
Acked-by: Sylwester Nawrocki <[email protected]>
---
drivers/phy/phy-exynos-mipi-video.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/phy/phy-exynos-mipi-video.c b/drivers/phy/phy-exynos-mipi-video.c
index 8b851f7..6bee04c 100644
--- a/drivers/phy/phy-exynos-mipi-video.c
+++ b/drivers/phy/phy-exynos-mipi-video.c
@@ -229,19 +229,6 @@ struct exynos_mipi_video_phy {
spinlock_t slock;
};

-static inline int __is_running(const struct exynos_mipi_phy_desc *data,
- struct exynos_mipi_video_phy *state)
-{
- u32 val;
- int ret;
-
- ret = regmap_read(state->regmaps[data->resetn_map], data->resetn_reg, &val);
- if (ret)
- return 0;
-
- return val & data->resetn_val;
-}
-
static int __set_phy_state(const struct exynos_mipi_phy_desc *data,
struct exynos_mipi_video_phy *state, unsigned int on)
{
@@ -251,7 +238,7 @@ static int __set_phy_state(const struct exynos_mipi_phy_desc *data,

/* disable in PMU sysreg */
if (!on && data->coupled_phy_id >= 0 &&
- !__is_running(state->phys[data->coupled_phy_id].data, state)) {
+ state->phys[data->coupled_phy_id].phy->power_count == 0) {
regmap_read(state->regmaps[data->enable_map], data->enable_reg,
&val);
val &= ~data->enable_val;
--
1.9.1


2016-11-07 18:50:45

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH RESEND] phy: exynos-mipi-video: simplify check for coupled phy status

Hello Marek,

On 11/07/2016 07:48 AM, Marek Szyprowski wrote:
> There is no need to access regmap of coupled phy to check its state - such
> information is already in the phy device itself, so use it directly. This
> let us to avoid possible access to registers of the device in the disabled
> power domain if the coupled phy is already disabled.
>

Do you think this patch needs also go to stable?

> Signed-off-by: Marek Szyprowski <[email protected]>
> Acked-by: Sylwester Nawrocki <[email protected]>
> ---

Reviewed-by: Javier Martinez Canillas <[email protected]>

Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America

2016-11-08 06:27:50

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH RESEND] phy: exynos-mipi-video: simplify check for coupled phy status

Hi Javier,


On 2016-11-07 19:50, Javier Martinez Canillas wrote:
> On 11/07/2016 07:48 AM, Marek Szyprowski wrote:
>> There is no need to access regmap of coupled phy to check its state - such
>> information is already in the phy device itself, so use it directly. This
>> let us to avoid possible access to registers of the device in the disabled
>> power domain if the coupled phy is already disabled.
>>
> Do you think this patch needs also go to stable?

Nope, there is no such need, because the problem with potential power domain
is specific for Exynos 5433, which is not yet merged to mainline.

>> Signed-off-by: Marek Szyprowski <[email protected]>
>> Acked-by: Sylwester Nawrocki <[email protected]>
>> ---
> Reviewed-by: Javier Martinez Canillas <[email protected]>

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

2016-11-08 11:35:28

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH RESEND] phy: exynos-mipi-video: simplify check for coupled phy status

Hello Marek,

On 11/08/2016 03:27 AM, Marek Szyprowski wrote:
> Hi Javier,
>
>
> On 2016-11-07 19:50, Javier Martinez Canillas wrote:
>> On 11/07/2016 07:48 AM, Marek Szyprowski wrote:
>>> There is no need to access regmap of coupled phy to check its state - such
>>> information is already in the phy device itself, so use it directly. This
>>> let us to avoid possible access to registers of the device in the disabled
>>> power domain if the coupled phy is already disabled.
>>>
>> Do you think this patch needs also go to stable?
>
> Nope, there is no such need, because the problem with potential power domain
> is specific for Exynos 5433, which is not yet merged to mainline.
>

Thanks for the clarification. It wasn't clear for me from the commit
message where did you face this issue.

>>> Signed-off-by: Marek Szyprowski <[email protected]>
>>> Acked-by: Sylwester Nawrocki <[email protected]>
>>> ---
>> Reviewed-by: Javier Martinez Canillas <[email protected]>
>
> Best regards

Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America

2016-11-15 13:33:48

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH RESEND] phy: exynos-mipi-video: simplify check for coupled phy status



On Monday 07 November 2016 04:18 PM, Marek Szyprowski wrote:
> There is no need to access regmap of coupled phy to check its state - such
> information is already in the phy device itself, so use it directly. This
> let us to avoid possible access to registers of the device in the disabled
> power domain if the coupled phy is already disabled.
>
> Signed-off-by: Marek Szyprowski <[email protected]>
> Acked-by: Sylwester Nawrocki <[email protected]>

merged to phy -next.

Thanks
Kishon
> ---
> drivers/phy/phy-exynos-mipi-video.c | 15 +--------------
> 1 file changed, 1 insertion(+), 14 deletions(-)
>
> diff --git a/drivers/phy/phy-exynos-mipi-video.c b/drivers/phy/phy-exynos-mipi-video.c
> index 8b851f7..6bee04c 100644
> --- a/drivers/phy/phy-exynos-mipi-video.c
> +++ b/drivers/phy/phy-exynos-mipi-video.c
> @@ -229,19 +229,6 @@ struct exynos_mipi_video_phy {
> spinlock_t slock;
> };
>
> -static inline int __is_running(const struct exynos_mipi_phy_desc *data,
> - struct exynos_mipi_video_phy *state)
> -{
> - u32 val;
> - int ret;
> -
> - ret = regmap_read(state->regmaps[data->resetn_map], data->resetn_reg, &val);
> - if (ret)
> - return 0;
> -
> - return val & data->resetn_val;
> -}
> -
> static int __set_phy_state(const struct exynos_mipi_phy_desc *data,
> struct exynos_mipi_video_phy *state, unsigned int on)
> {
> @@ -251,7 +238,7 @@ static int __set_phy_state(const struct exynos_mipi_phy_desc *data,
>
> /* disable in PMU sysreg */
> if (!on && data->coupled_phy_id >= 0 &&
> - !__is_running(state->phys[data->coupled_phy_id].data, state)) {
> + state->phys[data->coupled_phy_id].phy->power_count == 0) {
> regmap_read(state->regmaps[data->enable_map], data->enable_reg,
> &val);
> val &= ~data->enable_val;
>