The Crystal Cove PMIC used on some BYT/CHT devices has different revisions
when paired with Bay Trail (BYT) vs Cherry Trail (CHT) SoCs.
The current hardcoded values are only valid for CHT devices, change
the code so that it uses the correct register values on both BYT and CHT.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
.../media/atomisp/pci/atomisp_gmin_platform.c | 22 +++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
On Mon, Jan 23, 2023 at 01:51:28PM +0100, Hans de Goede wrote:
> The Crystal Cove PMIC used on some BYT/CHT devices has different revisions
> when paired with Bay Trail (BYT) vs Cherry Trail (CHT) SoCs.
>
> The current hardcoded values are only valid for CHT devices, change
> the code so that it uses the correct register values on both BYT and CHT.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
One nit-pick below.
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> .../media/atomisp/pci/atomisp_gmin_platform.c | 22 +++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> index 3d41fab661cf..6116d3c62315 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> @@ -57,8 +57,10 @@ enum clock_rate {
> #define LDO_1P8V_OFF 0x58 /* ... bottom bit is "enabled" */
>
> /* CRYSTAL COVE PMIC register set */
> -#define CRYSTAL_1P8V_REG 0x57
> -#define CRYSTAL_2P8V_REG 0x5d
> +#define CRYSTAL_BYT_1P8V_REG 0x5d
> +#define CRYSTAL_BYT_2P8V_REG 0x66
> +#define CRYSTAL_CHT_1P8V_REG 0x57
> +#define CRYSTAL_CHT_2P8V_REG 0x5d
> #define CRYSTAL_ON 0x63
> #define CRYSTAL_OFF 0x62
I would split to groups like this
#define CRYSTAL_BYT_1P8V_REG 0x5d
#define CRYSTAL_BYT_2P8V_REG 0x66
#define CRYSTAL_CHT_1P8V_REG 0x57
#define CRYSTAL_CHT_2P8V_REG 0x5d
#define CRYSTAL_ON 0x63
#define CRYSTAL_OFF 0x62
> @@ -843,6 +845,7 @@ static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on)
> struct gmin_subdev *gs = find_gmin_subdev(subdev);
> int ret;
> int value;
> + int reg;
>
> if (!gs || gs->v1p8_on == on)
> return 0;
> @@ -898,10 +901,15 @@ static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on)
> LDO10_REG, value, 0xff);
> break;
> case PMIC_CRYSTALCOVE:
> + if (IS_ISP2401)
> + reg = CRYSTAL_CHT_1P8V_REG;
> + else
> + reg = CRYSTAL_BYT_1P8V_REG;
> +
> value = on ? CRYSTAL_ON : CRYSTAL_OFF;
>
> ret = gmin_i2c_write(subdev->dev, gs->pwm_i2c_addr,
> - CRYSTAL_1P8V_REG, value, 0xff);
> + reg, value, 0xff);
> break;
> default:
> dev_err(subdev->dev, "Couldn't set power mode for v1p8\n");
> @@ -918,6 +926,7 @@ static int gmin_v2p8_ctrl(struct v4l2_subdev *subdev, int on)
> struct gmin_subdev *gs = find_gmin_subdev(subdev);
> int ret;
> int value;
> + int reg;
>
> if (WARN_ON(!gs))
> return -ENODEV;
> @@ -974,10 +983,15 @@ static int gmin_v2p8_ctrl(struct v4l2_subdev *subdev, int on)
> LDO9_REG, value, 0xff);
> break;
> case PMIC_CRYSTALCOVE:
> + if (IS_ISP2401)
> + reg = CRYSTAL_CHT_2P8V_REG;
> + else
> + reg = CRYSTAL_BYT_2P8V_REG;
> +
> value = on ? CRYSTAL_ON : CRYSTAL_OFF;
>
> ret = gmin_i2c_write(subdev->dev, gs->pwm_i2c_addr,
> - CRYSTAL_2P8V_REG, value, 0xff);
> + reg, value, 0xff);
> break;
> default:
> dev_err(subdev->dev, "Couldn't set power mode for v2p8\n");
> --
> 2.39.0
>
@@ -57,8 +57,10 @@ enum clock_rate {
#define LDO_1P8V_OFF 0x58 /* ... bottom bit is "enabled" */
/* CRYSTAL COVE PMIC register set */
-#define CRYSTAL_1P8V_REG 0x57
-#define CRYSTAL_2P8V_REG 0x5d
+#define CRYSTAL_BYT_1P8V_REG 0x5d
+#define CRYSTAL_BYT_2P8V_REG 0x66
+#define CRYSTAL_CHT_1P8V_REG 0x57
+#define CRYSTAL_CHT_2P8V_REG 0x5d
#define CRYSTAL_ON 0x63
#define CRYSTAL_OFF 0x62
@@ -843,6 +845,7 @@ static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on)
struct gmin_subdev *gs = find_gmin_subdev(subdev);
int ret;
int value;
+ int reg;
if (!gs || gs->v1p8_on == on)
return 0;
@@ -898,10 +901,15 @@ static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on)
LDO10_REG, value, 0xff);
break;
case PMIC_CRYSTALCOVE:
+ if (IS_ISP2401)
+ reg = CRYSTAL_CHT_1P8V_REG;
+ else
+ reg = CRYSTAL_BYT_1P8V_REG;
+
value = on ? CRYSTAL_ON : CRYSTAL_OFF;
ret = gmin_i2c_write(subdev->dev, gs->pwm_i2c_addr,
- CRYSTAL_1P8V_REG, value, 0xff);
+ reg, value, 0xff);
break;
default:
dev_err(subdev->dev, "Couldn't set power mode for v1p8\n");
@@ -918,6 +926,7 @@ static int gmin_v2p8_ctrl(struct v4l2_subdev *subdev, int on)
struct gmin_subdev *gs = find_gmin_subdev(subdev);
int ret;
int value;
+ int reg;
if (WARN_ON(!gs))
return -ENODEV;
@@ -974,10 +983,15 @@ static int gmin_v2p8_ctrl(struct v4l2_subdev *subdev, int on)
LDO9_REG, value, 0xff);
break;
case PMIC_CRYSTALCOVE:
+ if (IS_ISP2401)
+ reg = CRYSTAL_CHT_2P8V_REG;
+ else
+ reg = CRYSTAL_BYT_2P8V_REG;
+
value = on ? CRYSTAL_ON : CRYSTAL_OFF;
ret = gmin_i2c_write(subdev->dev, gs->pwm_i2c_addr,
- CRYSTAL_2P8V_REG, value, 0xff);
+ reg, value, 0xff);
break;
default:
dev_err(subdev->dev, "Couldn't set power mode for v2p8\n");