[PATCH v1 1/2] pinctrl: qcom: add driver for SA8775P SoC

[Caleb Connolly]

Hi Varadarajan,

On 3/24/25 09:05, Varadarajan Narayanan wrote:
> Add pinctrl and GPIO driver for SA8775P. Driver code is based on the
> similar U-Boot and Linux drivers.

Thanks, this mostly looks good.

[...]

> +static const unsigned int sa8775p_pin_offsets[] = {
> +	  [0] = SA8775_PIN_OFFSET,   [1] = SA8775_PIN_OFFSET,   [2] = SA8775_PIN_OFFSET,
> +	  [3] = SA8775_PIN_OFFSET,   [4] = SA8775_PIN_OFFSET,   [5] = SA8775_PIN_OFFSET,
> +	  [6] = SA8775_PIN_OFFSET,   [7] = SA8775_PIN_OFFSET,   [8] = SA8775_PIN_OFFSET,
> +	  [9] = SA8775_PIN_OFFSET,  [10] = SA8775_PIN_OFFSET,  [11] = SA8775_PIN_OFFSET,

I guess we're really hitting the limit with the current model, and 
should probably just aligned the API here with the Linux one (adding 
struct msm_pingroup, etc).

That's a fair bit of work which isn't fair to drop in your lap, and I 
don't really see a better way of solving this with the current API 
(though I do question if the devicetree is just wrong here?), so I'll 
take this as-is.

That all being said, I feel like some justification for this would have 
been good, either in the commit message (or below it with --- 
separator), certainly it would have saved me the time of 
reverse-engineering why it was done this way.

Reviewed-by: Caleb Connolly <caleb.connolly at linaro.org>

Kind regards,
> +	 [12] = SA8775_PIN_OFFSET,  [13] = SA8775_PIN_OFFSET,  [14] = SA8775_PIN_OFFSET,
> +	 [15] = SA8775_PIN_OFFSET,  [16] = SA8775_PIN_OFFSET,  [17] = SA8775_PIN_OFFSET,
> +	 [18] = SA8775_PIN_OFFSET,  [19] = SA8775_PIN_OFFSET,  [20] = SA8775_PIN_OFFSET,
> +	 [21] = SA8775_PIN_OFFSET,  [22] = SA8775_PIN_OFFSET,  [23] = SA8775_PIN_OFFSET,
> +	 [24] = SA8775_PIN_OFFSET,  [25] = SA8775_PIN_OFFSET,  [26] = SA8775_PIN_OFFSET,
> +	 [27] = SA8775_PIN_OFFSET,  [28] = SA8775_PIN_OFFSET,  [29] = SA8775_PIN_OFFSET,
> +	 [30] = SA8775_PIN_OFFSET,  [31] = SA8775_PIN_OFFSET,  [32] = SA8775_PIN_OFFSET,
> +	 [33] = SA8775_PIN_OFFSET,  [34] = SA8775_PIN_OFFSET,  [35] = SA8775_PIN_OFFSET,
> +	 [36] = SA8775_PIN_OFFSET,  [37] = SA8775_PIN_OFFSET,  [38] = SA8775_PIN_OFFSET,
> +	 [39] = SA8775_PIN_OFFSET,  [40] = SA8775_PIN_OFFSET,  [41] = SA8775_PIN_OFFSET,
> +	 [42] = SA8775_PIN_OFFSET,  [43] = SA8775_PIN_OFFSET,  [44] = SA8775_PIN_OFFSET,
> +	 [45] = SA8775_PIN_OFFSET,  [46] = SA8775_PIN_OFFSET,  [47] = SA8775_PIN_OFFSET,
> +	 [48] = SA8775_PIN_OFFSET,  [49] = SA8775_PIN_OFFSET,  [50] = SA8775_PIN_OFFSET,
> +	 [51] = SA8775_PIN_OFFSET,  [52] = SA8775_PIN_OFFSET,  [53] = SA8775_PIN_OFFSET,
> +	 [54] = SA8775_PIN_OFFSET,  [55] = SA8775_PIN_OFFSET,  [56] = SA8775_PIN_OFFSET,
> +	 [57] = SA8775_PIN_OFFSET,  [58] = SA8775_PIN_OFFSET,  [59] = SA8775_PIN_OFFSET,
> +	 [60] = SA8775_PIN_OFFSET,  [61] = SA8775_PIN_OFFSET,  [62] = SA8775_PIN_OFFSET,
> +	 [63] = SA8775_PIN_OFFSET,  [64] = SA8775_PIN_OFFSET,  [65] = SA8775_PIN_OFFSET,
> +	 [66] = SA8775_PIN_OFFSET,  [67] = SA8775_PIN_OFFSET,  [68] = SA8775_PIN_OFFSET,
> +	 [69] = SA8775_PIN_OFFSET,  [70] = SA8775_PIN_OFFSET,  [71] = SA8775_PIN_OFFSET,
> +	 [72] = SA8775_PIN_OFFSET,  [73] = SA8775_PIN_OFFSET,  [74] = SA8775_PIN_OFFSET,
> +	 [75] = SA8775_PIN_OFFSET,  [76] = SA8775_PIN_OFFSET,  [77] = SA8775_PIN_OFFSET,
> +	 [78] = SA8775_PIN_OFFSET,  [79] = SA8775_PIN_OFFSET,  [80] = SA8775_PIN_OFFSET,
> +	 [81] = SA8775_PIN_OFFSET,  [82] = SA8775_PIN_OFFSET,  [83] = SA8775_PIN_OFFSET,
> +	 [84] = SA8775_PIN_OFFSET,  [85] = SA8775_PIN_OFFSET,  [86] = SA8775_PIN_OFFSET,
> +	 [87] = SA8775_PIN_OFFSET,  [88] = SA8775_PIN_OFFSET,  [89] = SA8775_PIN_OFFSET,
> +	 [90] = SA8775_PIN_OFFSET,  [91] = SA8775_PIN_OFFSET,  [92] = SA8775_PIN_OFFSET,
> +	 [93] = SA8775_PIN_OFFSET,  [94] = SA8775_PIN_OFFSET,  [95] = SA8775_PIN_OFFSET,
> +	 [96] = SA8775_PIN_OFFSET,  [97] = SA8775_PIN_OFFSET,  [98] = SA8775_PIN_OFFSET,
> +	 [99] = SA8775_PIN_OFFSET, [100] = SA8775_PIN_OFFSET, [101] = SA8775_PIN_OFFSET,
> +	[102] = SA8775_PIN_OFFSET, [103] = SA8775_PIN_OFFSET, [104] = SA8775_PIN_OFFSET,
> +	[105] = SA8775_PIN_OFFSET, [106] = SA8775_PIN_OFFSET, [107] = SA8775_PIN_OFFSET,
> +	[108] = SA8775_PIN_OFFSET, [109] = SA8775_PIN_OFFSET, [110] = SA8775_PIN_OFFSET,
> +	[111] = SA8775_PIN_OFFSET, [112] = SA8775_PIN_OFFSET, [113] = SA8775_PIN_OFFSET,
> +	[114] = SA8775_PIN_OFFSET, [115] = SA8775_PIN_OFFSET, [116] = SA8775_PIN_OFFSET,
> +	[117] = SA8775_PIN_OFFSET, [118] = SA8775_PIN_OFFSET, [119] = SA8775_PIN_OFFSET,
> +	[120] = SA8775_PIN_OFFSET, [121] = SA8775_PIN_OFFSET, [122] = SA8775_PIN_OFFSET,
> +	[123] = SA8775_PIN_OFFSET, [124] = SA8775_PIN_OFFSET, [125] = SA8775_PIN_OFFSET,
> +	[126] = SA8775_PIN_OFFSET, [127] = SA8775_PIN_OFFSET, [128] = SA8775_PIN_OFFSET,
> +	[129] = SA8775_PIN_OFFSET, [130] = SA8775_PIN_OFFSET, [131] = SA8775_PIN_OFFSET,
> +	[132] = SA8775_PIN_OFFSET, [133] = SA8775_PIN_OFFSET, [134] = SA8775_PIN_OFFSET,
> +	[135] = SA8775_PIN_OFFSET, [136] = SA8775_PIN_OFFSET, [137] = SA8775_PIN_OFFSET,
> +	[138] = SA8775_PIN_OFFSET, [139] = SA8775_PIN_OFFSET, [140] = SA8775_PIN_OFFSET,
> +	[141] = SA8775_PIN_OFFSET, [142] = SA8775_PIN_OFFSET, [143] = SA8775_PIN_OFFSET,
> +	[144] = SA8775_PIN_OFFSET, [145] = SA8775_PIN_OFFSET, [146] = SA8775_PIN_OFFSET,
> +	[147] = SA8775_PIN_OFFSET, [148] = SA8775_PIN_OFFSET, [148] = SA8775_PIN_OFFSET,
> +	[149] = SA8775_PIN_OFFSET, [150] = SA8775_PIN_OFFSET, [151] = SA8775_PIN_OFFSET,
> +	[152] = SA8775_PIN_OFFSET, [153] = SA8775_PIN_OFFSET, [154] = SA8775_PIN_OFFSET,
> +};
> +
> +static const struct msm_pinctrl_data sa8775p_data = {
> +	.pin_data = {
> +		.pin_count = 155,
> +		.special_pins_start = 149,
> +		.special_pins_data = msm_special_pins_data,
> +		.pin_offsets = sa8775p_pin_offsets,
> +	},
> +	.functions_count = ARRAY_SIZE(msm_pinctrl_functions),
> +	.get_function_name = sa8775p_get_function_name,
> +	.get_function_mux = sa8775p_get_function_mux,
> +	.get_pin_name = sa8775p_get_pin_name,
> +};
> +
> +static const struct udevice_id msm_pinctrl_ids[] = {
> +	{ .compatible = "qcom,sa8775p-tlmm", .data = (ulong)&sa8775p_data },
> +	{ /* Sentinal */ }
> +};
> +
> +U_BOOT_DRIVER(pinctrl_sa8775p) = {
> +	.name           = "pinctrl_sa8775p",
> +	.id             = UCLASS_NOP,
> +	.of_match       = msm_pinctrl_ids,
> +	.ops            = &msm_pinctrl_ops,
> +	.bind           = msm_pinctrl_bind,
> +	.flags          = DM_FLAG_PRE_RELOC,
> +};
> 
> base-commit: 244e61fbb7f5045e4e187024f7ae80434c952145

-- 
Caleb (they/them)