Re: [PATCH v11 2/3] phy: qcom-snps: Add support for overriding phy tuning parameters

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 9/4/2022 8:47 PM, Vinod Koul wrote:
On 31-08-22, 18:41, Krishna Kurapati PSSNV wrote:
The ordering of this list needs to match the order of
override_param_map[] and there's nothing indicating this in the code.

I was considering suggesting that you add a enum/define and do
	[SQUELCH_DETECTOR_BP] = "qcom,squelch-detector-bp",
	...
and then do the same in the override_param_map array.

But I think it will be cleaner if you add a const char const pointer to
override_param_map and just specify these strings in the
override_param_map array.

Each entry will grow by a pointer, but multiple copies of the same
strings (when added in the future) should be combined by the compiler.

IIUC, you want me to remove this array of const char*'s and embed them in
the override_param_map and iterate through it without using this const
phy_seq_props as a reference.

I think that would make it simpler..

+static const struct override_param_map sc7280[] = {

There's nothing ensuring that the loop below doesn't run off the end of
this array. So when the next platform is added, there's no way to
handle the fact that they might have a different set of properties.

If you add the property name to these elements, that will no longer be a
problem (and you can add a {} entry at the end of the list and check for
this when looping over the elements.

Would be great if this is addressed as well
Hi Vinod. Thanks for the review. I have posted v12 addressing these comments.

https://lore.kernel.org/linux-usb/1662201048-26049-1-git-send-email-quic_kriskura@xxxxxxxxxxx/

Regards,
Krishna,




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux