Hi Bryan,
On 8/7/2024 11:37 PM, Bryan O'Donoghue wrote:
On 07/08/2024 16:03, Depeng Shao wrote:
Hi Bryan,
On 8/7/2024 10:04 PM, Bryan O'Donoghue wrote:
On 07/08/2024 14:08, Depeng Shao wrote:
Hi Vladimir,
On 8/5/2024 5:26 AM, Vladimir Zapolskiy wrote:
Hi Bryan,
On 8/1/24 11:16, Bryan O'Donoghue wrote:
On 01/08/2024 00:43, Vladimir Zapolskiy wrote:
+ ret = csiphy->res->hw_ops->init(csiphy);
Here.
What name would make more sense to you ?
according to the implementation the .init() call just fills some
data in
memory, so I believe this could be handled at build time, if it's done
carefully enough...
This camss-csiphy-3ph-1-0.c is reused by many platforms, the old
platforms have same CSI_COMMON_CTR register offset, their offset are
0x800, but some new platforms may have different CSI_COMMON_CTR
register offset, for example, the CSI_COMMON_CTR register offset is
0x1000 in sm8550, then we need to add new file to support the new
csiphy HW, e.g., camss-csiphy-3ph-2-0.c, so Bryan asked me to
develop the CSIPHY driver based on his changes, then we just need
few code to enable new CSIPHY.
Regarding the hw_ops->init interface, since it fills HW register
configurations and HW register offset, then maybe, it also can be
called as HW operation.
And looks like we can't move it to camss-csiphy.c since it does
platform specific operation and it is related to the registers.
Please feel free to share other comments if you don't agree with it.
Thanks.
Thanks,
Depeng
So, I agree the phy init data could be obtained via resource structs
but, rather than add yet more patches to this series, I'd say we can
make the move to a separate resource struct pointer at a later date.
Lets drop this patch and @Depeng we can then do
+ regs->offset = 0x800;
media: qcom: camss: csiphy-3ph: Use an offset variable to find common
control regs
Do you mean only drop "[PATCH 04/13] media: qcom: camss: csiphy: Add
an init callback to CSI PHY devices"?
[PATCH 05/13] media: qcom: camss: csiphy-3ph: Move CSIPHY variables to
data field inside csiphy struct
Do you mean this is still needed? Just don't move the code from
csiphy_gen2_config_lanes to csiphy_init, right?
[PATCH 06/13] media: qcom: camss: csiphy-3ph: Use an offset variable
to find common control regs
The offset change is also needed, just need to add the offset for
different platform in csiphy_gen2_config_lanes .
Please correct me if my understanding is wrong. Thanks.
Correct.
I'm updating the code based on above comments, but I meet crash issue if
I move the offset assignment to csiphy_gen2_config_lanes, since the
csiphy->res->hw_ops->reset(csiphy) is called earlier than
csiphy_gen2_config_lanes, so if we don't have the .init interface, we
only can move this offset value to `struct csiphy_subdev_resources`, but
if we add the offset to `struct csiphy_subdev_resources`, then below two
patches are also can be dropped.
[PATCH 05/13] media: qcom: camss: csiphy-3ph: Move CSIPHY variables to
data field inside csiphy struct
[PATCH 06/13] media: qcom: camss: csiphy-3ph: Use an offset variable to
find common control regs
Could you please comment on if I need to add the CSI_COMMON_CTR offset
to res directly?
Or add back the .init interface?
---
[ 43.162439] Unable to handle kernel NULL pointer dereference at
virtual address 000000000000000c
[ 43.428307] Call trace:
[ 43.430823] csiphy_reset+0x28/0x60 [qcom_camss]
[ 43.435572] csiphy_set_power+0x1e8/0x2d4 [qcom_camss]
[ 43.440846] pipeline_pm_power_one+0x74/0x10c [videodev]
[ 43.446306] pipeline_pm_power+0x44/0xe0 [videodev]
[ 43.451313] v4l2_pipeline_pm_get+0x44/0x80 [videodev]
[ 43.456588] video_open+0x6c/0xc4 [qcom_camss]
[ 43.461158] v4l2_open+0xb8/0x100 [videodev]
[ 43.465549] chrdev_open+0x174/0x208
[ 43.469224] do_dentry_open+0x290/0x4b4
[ 43.473164] vfs_open+0x30/0xf0
[ 43.476397] path_openat+0xaec/0xd2c
[ 43.480069] do_filp_open+0xb4/0x158
[ 43.483739] do_sys_openat2+0x84/0xe8
[ 43.487500] __arm64_sys_openat+0x70/0x98
[ 43.491619] invoke_syscall+0x40/0xf8
[ 43.495383] el0_svc_common+0xa8/0xd8
[ 43.499143] do_el0_svc+0x1c/0x28
[ 43.502545] el0_svc+0x38/0x68
[ 43.505691] el0t_64_sync_handler+0x90/0xfc
[ 43.509989] el0t_64_sync+0x190/0x194
[ 43.513751] Code: 52800028 aa0003f3 52827100 5283e801 (b9400e8a)
[ 43.520010] ---[ end trace 0000000000000000 ]---
Segmentation fault
Thanks,
Depeng
