diff mbox series

[v6,1/4] ACPI / PMIC: Add support for executing PMIC MIPI sequence elements

Message ID 20190107111556.4510-2-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show
Series ACPI/i915: Add support for PMIC MIPI sequence elements | expand

Commit Message

Hans de Goede Jan. 7, 2019, 11:15 a.m. UTC 
DSI LCD panels describe an initialization sequence in the Video BIOS
Tables using so called MIPI sequences. One possible element in these
sequences is a PMIC specific element of 15 bytes.

Although this is not really an ACPI opregion, the ACPI opregion code is the
closest thing we have. We need to have support for these PMIC specific MIPI
sequence elements somwhere. Since we already instantiate a special platform
device for Intel PMICs for the ACPI PMIC OpRegion handler to bind to,
with PMIC specific implementations of the OpRegion, the handling of MIPI
sequence PMIC elements fits very well in the ACPI PMIC OpRegion code.

This commit adds a new intel_soc_pmic_exec_mipi_pmic_seq_element()
function, which is to be backed by a PMIC specific
exec_mipi_pmic_seq_element callback. This function will be called by the
i915 code to execture MIPI sequence PMIC elements.

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v6:
-Small refactoring of intel_soc_pmic_exec_mipi_pmic_seq_element to make
 adding a generic implementation later easier

Changes in v5:
-Pass i2c-address + register-address + value + mask as separate arguments to
 the intel_soc_pmic_exec_mipi_pmic_seq_element function. Instead of passing
 the 15 bytes of raw MIPI sequence data. The decoding will be done in the
 i915 VBT code instead.

Changes in v3:
-Add kerneldoc for intel_soc_pmic_exec_mipi_pmic_seq_element
-Make intel_soc_pmic_exec_mipi_pmic_seq_element return errors
---
 drivers/acpi/pmic/intel_pmic.c     | 52 ++++++++++++++++++++++++++++++
 drivers/acpi/pmic/intel_pmic.h     |  2 ++
 include/linux/mfd/intel_soc_pmic.h |  3 ++
 3 files changed, 57 insertions(+)

Comments

Andy Shevchenko Jan. 7, 2019, 3:35 p.m. UTC | #1 
On Mon, Jan 07, 2019 at 12:15:53PM +0100, Hans de Goede wrote:
> DSI LCD panels describe an initialization sequence in the Video BIOS
> Tables using so called MIPI sequences. One possible element in these
> sequences is a PMIC specific element of 15 bytes.
> 
> Although this is not really an ACPI opregion, the ACPI opregion code is the
> closest thing we have. We need to have support for these PMIC specific MIPI
> sequence elements somwhere. Since we already instantiate a special platform
> device for Intel PMICs for the ACPI PMIC OpRegion handler to bind to,
> with PMIC specific implementations of the OpRegion, the handling of MIPI
> sequence PMIC elements fits very well in the ACPI PMIC OpRegion code.
> 
> This commit adds a new intel_soc_pmic_exec_mipi_pmic_seq_element()
> function, which is to be backed by a PMIC specific
> exec_mipi_pmic_seq_element callback. This function will be called by the
> i915 code to execture MIPI sequence PMIC elements.

> +/**
> + * intel_soc_pmic_exec_mipi_pmic_seq_element - Execute PMIC MIPI sequence

I wonder if we need pmic duplication in the name.

> + * @i2c_address:  I2C client address for the PMIC
> + * @reg_address:  PMIC register address
> + * @value:        New value for the register bits to change
> + * @mask:         Mask indicating which register bits to change
> + *
> + * DSI LCD panels describe an initialization sequence in the i915 VBT (Video
> + * BIOS Tables) using so called MIPI sequences. One possible element in these
> + * sequences is a PMIC specific element of 15 bytes.
> + *
> + * This function executes these PMIC specific elements sending the embedded
> + * commands to the PMIC.
> + *
> + * Return 0 on success, < 0 on failure.
> + */
> +int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
> +					      u32 value, u32 mask)
> +{
> +	struct intel_pmic_opregion_data *d;
> +	int ret;
> +
> +	if (!intel_pmic_opregion) {
> +		pr_warn("%s: No PMIC registered\n", __func__);
> +		return -ENXIO;
> +	}
> +
> +	d = intel_pmic_opregion->data;
> +
> +	mutex_lock(&intel_pmic_opregion->lock);
> +
> +	if (d->exec_mipi_pmic_seq_element) {

> +		ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap,
> +						    i2c_address, reg_address,
> +						    value, mask);

Here it's not quite a dup, but it's implied by the name of structure...

> +	} else {
> +		pr_warn("%s: Not implemented\n", __func__);
> +		pr_warn("%s: i2c-addr: 0x%x reg-addr 0x%x value 0x%x mask 0x%x\n",
> +			__func__, i2c_address, reg_address, value, mask);
> +		ret = -EOPNOTSUPP;
> +	}
> +
> +	mutex_unlock(&intel_pmic_opregion->lock);
> +
> +	return ret;
> +}
Hans de Goede Jan. 8, 2019, 1:40 p.m. UTC | #2 
Hi,

On 07-01-19 16:35, Andy Shevchenko wrote:
> On Mon, Jan 07, 2019 at 12:15:53PM +0100, Hans de Goede wrote:
>> DSI LCD panels describe an initialization sequence in the Video BIOS
>> Tables using so called MIPI sequences. One possible element in these
>> sequences is a PMIC specific element of 15 bytes.
>>
>> Although this is not really an ACPI opregion, the ACPI opregion code is the
>> closest thing we have. We need to have support for these PMIC specific MIPI
>> sequence elements somwhere. Since we already instantiate a special platform
>> device for Intel PMICs for the ACPI PMIC OpRegion handler to bind to,
>> with PMIC specific implementations of the OpRegion, the handling of MIPI
>> sequence PMIC elements fits very well in the ACPI PMIC OpRegion code.
>>
>> This commit adds a new intel_soc_pmic_exec_mipi_pmic_seq_element()
>> function, which is to be backed by a PMIC specific
>> exec_mipi_pmic_seq_element callback. This function will be called by the
>> i915 code to execture MIPI sequence PMIC elements.
> 
>> +/**
>> + * intel_soc_pmic_exec_mipi_pmic_seq_element - Execute PMIC MIPI sequence
> 
> I wonder if we need pmic duplication in the name.

Mipi sequences can do a bunch of things, talk to the panel over
the MIPI bus, set GPIOs, read-modify-write PMIC registers, etc.

This function can only execute the PMIC bits, not the rest,
so the second pmic is there to indiciate we are executing
a PMIC MIPI sequence and not e.g. a GPIO one. So I believe that
keeping this as is is best.

Regards,

Hans


>> + * @i2c_address:  I2C client address for the PMIC
>> + * @reg_address:  PMIC register address
>> + * @value:        New value for the register bits to change
>> + * @mask:         Mask indicating which register bits to change
>> + *
>> + * DSI LCD panels describe an initialization sequence in the i915 VBT (Video
>> + * BIOS Tables) using so called MIPI sequences. One possible element in these
>> + * sequences is a PMIC specific element of 15 bytes.
>> + *
>> + * This function executes these PMIC specific elements sending the embedded
>> + * commands to the PMIC.
>> + *
>> + * Return 0 on success, < 0 on failure.
>> + */
>> +int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
>> +					      u32 value, u32 mask)
>> +{
>> +	struct intel_pmic_opregion_data *d;
>> +	int ret;
>> +
>> +	if (!intel_pmic_opregion) {
>> +		pr_warn("%s: No PMIC registered\n", __func__);
>> +		return -ENXIO;
>> +	}
>> +
>> +	d = intel_pmic_opregion->data;
>> +
>> +	mutex_lock(&intel_pmic_opregion->lock);
>> +
>> +	if (d->exec_mipi_pmic_seq_element) {
> 
>> +		ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap,
>> +						    i2c_address, reg_address,
>> +						    value, mask);
> 
> Here it's not quite a dup, but it's implied by the name of structure...
> 
>> +	} else {
>> +		pr_warn("%s: Not implemented\n", __func__);
>> +		pr_warn("%s: i2c-addr: 0x%x reg-addr 0x%x value 0x%x mask 0x%x\n",
>> +			__func__, i2c_address, reg_address, value, mask);
>> +		ret = -EOPNOTSUPP;
>> +	}
>> +
>> +	mutex_unlock(&intel_pmic_opregion->lock);
>> +
>> +	return ret;
>> +}
>
diff mbox series

Patch

diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
index ca18e0d23df9..471afeea87c2 100644
--- a/drivers/acpi/pmic/intel_pmic.c
+++ b/drivers/acpi/pmic/intel_pmic.c
@@ -15,6 +15,7 @@ 
 
 #include <linux/export.h>
 #include <linux/acpi.h>
+#include <linux/mfd/intel_soc_pmic.h>
 #include <linux/regmap.h>
 #include <acpi/acpi_lpat.h>
 #include "intel_pmic.h"
@@ -36,6 +37,8 @@  struct intel_pmic_opregion {
 	struct intel_pmic_regs_handler_ctx ctx;
 };
 
+static struct intel_pmic_opregion *intel_pmic_opregion;
+
 static int pmic_get_reg_bit(int address, struct pmic_table *table,
 			    int count, int *reg, int *bit)
 {
@@ -304,6 +307,7 @@  int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
 	}
 
 	opregion->data = d;
+	intel_pmic_opregion = opregion;
 	return 0;
 
 out_remove_thermal_handler:
@@ -319,3 +323,51 @@  int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(intel_pmic_install_opregion_handler);
+
+/**
+ * intel_soc_pmic_exec_mipi_pmic_seq_element - Execute PMIC MIPI sequence
+ * @i2c_address:  I2C client address for the PMIC
+ * @reg_address:  PMIC register address
+ * @value:        New value for the register bits to change
+ * @mask:         Mask indicating which register bits to change
+ *
+ * DSI LCD panels describe an initialization sequence in the i915 VBT (Video
+ * BIOS Tables) using so called MIPI sequences. One possible element in these
+ * sequences is a PMIC specific element of 15 bytes.
+ *
+ * This function executes these PMIC specific elements sending the embedded
+ * commands to the PMIC.
+ *
+ * Return 0 on success, < 0 on failure.
+ */
+int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
+					      u32 value, u32 mask)
+{
+	struct intel_pmic_opregion_data *d;
+	int ret;
+
+	if (!intel_pmic_opregion) {
+		pr_warn("%s: No PMIC registered\n", __func__);
+		return -ENXIO;
+	}
+
+	d = intel_pmic_opregion->data;
+
+	mutex_lock(&intel_pmic_opregion->lock);
+
+	if (d->exec_mipi_pmic_seq_element) {
+		ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap,
+						    i2c_address, reg_address,
+						    value, mask);
+	} else {
+		pr_warn("%s: Not implemented\n", __func__);
+		pr_warn("%s: i2c-addr: 0x%x reg-addr 0x%x value 0x%x mask 0x%x\n",
+			__func__, i2c_address, reg_address, value, mask);
+		ret = -EOPNOTSUPP;
+	}
+
+	mutex_unlock(&intel_pmic_opregion->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(intel_soc_pmic_exec_mipi_pmic_seq_element);
diff --git a/drivers/acpi/pmic/intel_pmic.h b/drivers/acpi/pmic/intel_pmic.h
index 095afc96952e..5cd195fabca8 100644
--- a/drivers/acpi/pmic/intel_pmic.h
+++ b/drivers/acpi/pmic/intel_pmic.h
@@ -15,6 +15,8 @@  struct intel_pmic_opregion_data {
 	int (*update_aux)(struct regmap *r, int reg, int raw_temp);
 	int (*get_policy)(struct regmap *r, int reg, int bit, u64 *value);
 	int (*update_policy)(struct regmap *r, int reg, int bit, int enable);
+	int (*exec_mipi_pmic_seq_element)(struct regmap *r, u16 i2c_address,
+					  u32 reg_address, u32 value, u32 mask);
 	struct pmic_table *power_table;
 	int power_table_count;
 	struct pmic_table *thermal_table;
diff --git a/include/linux/mfd/intel_soc_pmic.h b/include/linux/mfd/intel_soc_pmic.h
index ed1dfba5e5f9..bfecd6bd4990 100644
--- a/include/linux/mfd/intel_soc_pmic.h
+++ b/include/linux/mfd/intel_soc_pmic.h
@@ -26,4 +26,7 @@  struct intel_soc_pmic {
 	struct device *dev;
 };
 
+int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
+					      u32 value, u32 mask);
+
 #endif	/* __INTEL_SOC_PMIC_H__ */