[05/13] media: qcom: camss: csiphy-3ph: Move CSIPHY variables to data field inside csiphy struct
Commit Message
From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
A .data field in the csiphy device structure allows us to extend out the
register layout of the three phase capable CSIPHY layer.
Move the existing lane configuration structure to an encapsulating
structure -> struct csiphy_device_regs which is derived from the .data
field populated at PHY init time, as opposed to calculated at lane
configuration.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Depeng Shao <quic_depengs@quicinc.com>
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
.../qcom/camss/camss-csiphy-3ph-1-0.c | 55 ++++++++++++-------
.../media/platform/qcom/camss/camss-csiphy.h | 1 +
2 files changed, 36 insertions(+), 20 deletions(-)
Comments
On 8/12/24 17:41, Depeng Shao wrote:
> From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>
> A .data field in the csiphy device structure allows us to extend out the
> register layout of the three phase capable CSIPHY layer.
>
> Move the existing lane configuration structure to an encapsulating
> structure -> struct csiphy_device_regs which is derived from the .data
> field populated at PHY init time, as opposed to calculated at lane
> configuration.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Signed-off-by: Depeng Shao <quic_depengs@quicinc.com>
> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
> .../qcom/camss/camss-csiphy-3ph-1-0.c | 55 ++++++++++++-------
> .../media/platform/qcom/camss/camss-csiphy.h | 1 +
> 2 files changed, 36 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> index b60c32a195df..93782ebfe0ea 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
> @@ -63,6 +63,11 @@ struct csiphy_lane_regs {
> u32 csiphy_param_type;
> };
>
> +struct csiphy_device_regs {
> + const struct csiphy_lane_regs *lane_regs;
> + int lane_array_size;
> +};
> +
> /* GEN2 1.0 2PH */
> static const struct
> csiphy_lane_regs lane_regs_sdm845[] = {
> @@ -470,28 +475,11 @@ static void csiphy_gen1_config_lanes(struct csiphy_device *csiphy,
> static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy,
> u8 settle_cnt)
> {
> - const struct csiphy_lane_regs *r;
> - int i, array_size;
> + struct csiphy_device_regs *csiphy_regs = csiphy->data;
> + const struct csiphy_lane_regs *r = csiphy_regs->lane_regs;
> + int i, array_size = csiphy_regs->lane_array_size;
> u32 val;
>
> - switch (csiphy->camss->res->version) {
> - case CAMSS_845:
> - r = &lane_regs_sdm845[0];
> - array_size = ARRAY_SIZE(lane_regs_sdm845);
> - break;
> - case CAMSS_8250:
> - r = &lane_regs_sm8250[0];
> - array_size = ARRAY_SIZE(lane_regs_sm8250);
> - break;
> - case CAMSS_8280XP:
> - r = &lane_regs_sc8280xp[0];
> - array_size = ARRAY_SIZE(lane_regs_sc8280xp);
> - break;
> - default:
> - WARN(1, "unknown cspi version\n");
> - return;
> - }
> -
> for (i = 0; i < array_size; i++, r++) {
> switch (r->csiphy_param_type) {
> case CSIPHY_SETTLE_CNT_LOWER_BYTE:
> @@ -583,6 +571,33 @@ static void csiphy_lanes_disable(struct csiphy_device *csiphy,
>
> static int csiphy_init(struct csiphy_device *csiphy)
> {
> + struct device *dev = csiphy->camss->dev;
> + struct csiphy_device_regs *regs;
> +
> + regs = devm_kmalloc(dev, sizeof(*regs), GFP_KERNEL);
> + if (!regs)
> + return -ENOMEM;
> +
> + csiphy->data = regs;
> +
> + switch (csiphy->camss->res->version) {
> + case CAMSS_845:
> + regs->lane_regs = &lane_regs_sdm845[0];
> + regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
> + break;
> + case CAMSS_8250:
> + regs->lane_regs = &lane_regs_sm8250[0];
> + regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8250);
> + break;
> + case CAMSS_8280XP:
> + regs->lane_regs = &lane_regs_sc8280xp[0];
> + regs->lane_array_size = ARRAY_SIZE(lane_regs_sc8280xp);
> + break;
> + default:
> + WARN(1, "unknown csiphy version\n");
> + return -ENODEV;
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
> index bdf9a9c8bacc..cac1f800b7d8 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.h
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
> @@ -95,6 +95,7 @@ struct csiphy_device {
> struct csiphy_config cfg;
> struct v4l2_mbus_framefmt fmt[MSM_CSIPHY_PADS_NUM];
> const struct csiphy_subdev_resources *res;
> + void *data;
I would suggest to make the type/name above explicit:
struct csiphy_device_regs *regs;
> };
>
> struct camss_subdev_resources;
--
Best wishes,
Vladimir
Hi Bryan,
On 8/19/2024 8:01 AM, Vladimir Zapolskiy wrote:
> On 8/12/24 17:41, Depeng Shao wrote:
>> From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>
>> A .data field in the csiphy device structure allows us to extend out the
>> register layout of the three phase capable CSIPHY layer.
>>
>> Move the existing lane configuration structure to an encapsulating
>> structure -> struct csiphy_device_regs which is derived from the .data
>> field populated at PHY init time, as opposed to calculated at lane
>> configuration.
>>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> Signed-off-by: Depeng Shao <quic_depengs@quicinc.com>
>> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> ---
>> .../qcom/camss/camss-csiphy-3ph-1-0.c | 55 ++++++++++++-------
>> .../media/platform/qcom/camss/camss-csiphy.h | 1 +
>> 2 files changed, 36 insertions(+), 20 deletions(-)
>> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/
>> drivers/media/platform/qcom/camss/camss-csiphy.h
>> index bdf9a9c8bacc..cac1f800b7d8 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csiphy.h
>> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
>> @@ -95,6 +95,7 @@ struct csiphy_device {
>> struct csiphy_config cfg;
>> struct v4l2_mbus_framefmt fmt[MSM_CSIPHY_PADS_NUM];
>> const struct csiphy_subdev_resources *res;
>> + void *data;
>
> I would suggest to make the type/name above explicit:
>
I will follow Vladimir's suggestion to update the type/name, please
reply this mail if you have other comment.
Thanks,
Depeng
@@ -63,6 +63,11 @@ struct csiphy_lane_regs {
u32 csiphy_param_type;
};
+struct csiphy_device_regs {
+ const struct csiphy_lane_regs *lane_regs;
+ int lane_array_size;
+};
+
/* GEN2 1.0 2PH */
static const struct
csiphy_lane_regs lane_regs_sdm845[] = {
@@ -470,28 +475,11 @@ static void csiphy_gen1_config_lanes(struct csiphy_device *csiphy,
static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy,
u8 settle_cnt)
{
- const struct csiphy_lane_regs *r;
- int i, array_size;
+ struct csiphy_device_regs *csiphy_regs = csiphy->data;
+ const struct csiphy_lane_regs *r = csiphy_regs->lane_regs;
+ int i, array_size = csiphy_regs->lane_array_size;
u32 val;
- switch (csiphy->camss->res->version) {
- case CAMSS_845:
- r = &lane_regs_sdm845[0];
- array_size = ARRAY_SIZE(lane_regs_sdm845);
- break;
- case CAMSS_8250:
- r = &lane_regs_sm8250[0];
- array_size = ARRAY_SIZE(lane_regs_sm8250);
- break;
- case CAMSS_8280XP:
- r = &lane_regs_sc8280xp[0];
- array_size = ARRAY_SIZE(lane_regs_sc8280xp);
- break;
- default:
- WARN(1, "unknown cspi version\n");
- return;
- }
-
for (i = 0; i < array_size; i++, r++) {
switch (r->csiphy_param_type) {
case CSIPHY_SETTLE_CNT_LOWER_BYTE:
@@ -583,6 +571,33 @@ static void csiphy_lanes_disable(struct csiphy_device *csiphy,
static int csiphy_init(struct csiphy_device *csiphy)
{
+ struct device *dev = csiphy->camss->dev;
+ struct csiphy_device_regs *regs;
+
+ regs = devm_kmalloc(dev, sizeof(*regs), GFP_KERNEL);
+ if (!regs)
+ return -ENOMEM;
+
+ csiphy->data = regs;
+
+ switch (csiphy->camss->res->version) {
+ case CAMSS_845:
+ regs->lane_regs = &lane_regs_sdm845[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845);
+ break;
+ case CAMSS_8250:
+ regs->lane_regs = &lane_regs_sm8250[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8250);
+ break;
+ case CAMSS_8280XP:
+ regs->lane_regs = &lane_regs_sc8280xp[0];
+ regs->lane_array_size = ARRAY_SIZE(lane_regs_sc8280xp);
+ break;
+ default:
+ WARN(1, "unknown csiphy version\n");
+ return -ENODEV;
+ }
+
return 0;
}
@@ -95,6 +95,7 @@ struct csiphy_device {
struct csiphy_config cfg;
struct v4l2_mbus_framefmt fmt[MSM_CSIPHY_PADS_NUM];
const struct csiphy_subdev_resources *res;
+ void *data;
};
struct camss_subdev_resources;