Re: [PATCH v2 3/8] media: platform: amd: Add helpers to configure isp4 mipi phy

[Sakari Ailus]

Hi Bin,

On Wed, Jun 18, 2025 at 05:19:54PM +0800, Bin Du wrote:
> +struct isp4phy_mipi_reg_seq {
> +	u16 addr;
> +	u16 mask;
> +	u16 data;
> +};
> +
> +union isp4phy_mipi_0 {
> +	struct {
> +		u32 shutdownz : 1;
> +		u32 rstz : 1;
> +	} bit;
> +	u32 value;
> +};
> +
> +union isp4phy_mipi_1 {
> +	struct {
> +		u32 mode : 1;
> +	} bit;
> +	u32 value;
> +};
> +
> +union isp4phy_mipi_2 {
> +	struct {
> +		u32 rxdatawidthhs_0 : 2;
> +		u32 rxdatawidthhs_1 : 2;
> +		u32 rxdatawidthhs_2 : 2;
> +		u32 rxdatawidthhs_3 : 2;
> +	} bit;
> +	u32 value;
> +};
> +
> +struct isp4phy_mipi_3 {
> +	u32 reserved;
> +};
> +
> +union isp4phy_mipi_4 {
> +	struct {
> +		u32 enableclk : 1;
> +		u32 enable_0 : 1;
> +		u32 enable_1 : 1;
> +		u32 enable_2 : 1;
> +		u32 enable_3 : 1;
> +	} bit;
> +	u32 value;
> +};
> +
> +union isp4phy_mipi_5 {
> +	struct {
> +		u32 forcerxmode_0 : 1;
> +		u32 forcerxmode_1 : 1;
> +		u32 forcerxmode_2 : 1;
> +		u32 forcerxmode_3 : 1;
> +		u32 forcerxmode_clk : 1;
> +	} bit;
> +	u32 value;
> +};
> +
> +union isp4phy_mipi_6 {
> +	struct {
> +		u32 turndisable_0 : 1;
> +		u32 turndisable_1 : 1;
> +		u32 turndisable_2 : 1;
> +		u32 turndisable_3 : 1;
> +	} bit;
> +	u32 value;
> +};
> +
> +union isp4phy_mipi_7 {
> +	struct {
> +		u32 ready : 1;
> +	} bit;
> +	u32 value;
> +};
> +
> +union isp4phy_mipi_ind_idx {
> +	struct {
> +		u32 addr : 16;
> +	} bit;
> +	u32 value;
> +};
> +
> +union isp4phy_mipi_ind_data {
> +	struct {
> +		u32 data : 16;
> +	} bit;
> +	u32 value;
> +};
> +
> +union isp4phy_mipi_ind_wack {
> +	struct {
> +		u32 ack : 1;
> +		u32 pslverr : 1;
> +	} bit;
> +	u32 value;
> +};
> +
> +struct isp4phy_mipi_reg {
> +	union isp4phy_mipi_0 isp_mipi_phy0;
> +	union isp4phy_mipi_1 isp_mipi_phy1;
> +	union isp4phy_mipi_2 isp_mipi_phy2;
> +	struct isp4phy_mipi_3 isp_mipi_phy3;
> +	union isp4phy_mipi_4 isp_mipi_phy4;
> +	union isp4phy_mipi_5 isp_mipi_phy5;
> +	union isp4phy_mipi_6 isp_mipi_phy6;
> +	union isp4phy_mipi_7 isp_mipi_phy7;
> +	u32 reserve;
> +	union isp4phy_mipi_ind_idx isp_mipi_phy_ind_idx;
> +	union isp4phy_mipi_ind_data isp_mipi_phy_ind_data;
> +	union isp4phy_mipi_ind_wack isp_mipi_phy_inc_wack;
> +};

One more thing. Is there an endianness issue here?

Overall the CPU endianness may be big or little while the device endianness
is little, presumably. You should use __le* types and functions to convert
the endianness when dealing with the messages to and from the device.

-- 
Kind regards,

Sakari Ailus