Messages in this thread | | | Date | Wed, 15 Mar 2023 19:07:30 +0530 | From | Manivannan Sadhasivam <> | Subject | Re: [PATCH v3] iommu/arm-smmu-qcom: Rework the logic finding the bypass quirk |
| |
On Wed, Mar 15, 2023 at 09:38:42AM +0100, Johan Hovold wrote: > On Wed, Mar 15, 2023 at 01:29:58PM +0530, Manivannan Sadhasivam wrote: > > On Wed, Mar 15, 2023 at 08:37:32AM +0100, Johan Hovold wrote: > > > > > +static int qcom_smmu_cfg_probe(struct arm_smmu_device *smmu) > > > > +{ > > > > + u32 smr; > > > > + int i; > > > > + > > > > + /* > > > > + * Limit the number of stream matching groups to 128 as the ARM SMMU architecture > > > > + * specification defines NUMSMRG (Number of Stream Mapping Register Groups) in the > > > > + * range of 0-127, but some Qcom platforms emulate more stream mapping groups. And > > > > + * those groups don't exhibit the same behavior as the architecture supported ones. > > > > + */ > > > > > > Please fix your editor so that it wraps lines at 80 columns, which is > > > still the preferred (soft) limit. > > > > > > > If exceeding 80 columns end up making the comment more readable (fewer lines), > > then why should we limit ourselves? > > Exceeding 80 column for comments does generally not improve readability. >
It all depends on the perspective/preference... But I can limit to 80 columns here.
> That part of the coding standard has do to with not adding excessive > line breaks to *code*, where it can sometimes impact readability. > > > > > + if (smmu->num_mapping_groups > 128) { > > > > + dev_warn(smmu->dev, "\tLimiting the stream matching groups to 128\n"); > > > > > > dev_notice() should do since there's nothing a user can do about this. > > > > > > > Ok. > > > > > > + smmu->num_mapping_groups = 128; > > > > + } > > > > > > So this hunk is really all that is needed to make the current quirk > > > detection work on sc8280xp. Why not simply stick with the current logic > > > and use the last group until there is a need for anything more? > > > > > > > No! What if the bootloader had set up mapping for 128 groups? In that case > > we'll overwrite the last group. It is still required to find the valid group > > and use it for quirk detection. If no group is available, we'll skip it. > > Yes, but that's also entirely hypothetical (and could perhaps also be > handled by adding a warning for now). > > If you want to rework the quirk handling for this you should at least do > so in a separate patch as it is arguably a separate change from fixing > the current quirk detection for newer SoCs by capping the number of > groups (a minimal fix that could be backported). >
Using a separate patch makes sense to me. Will do that in v4.
Thanks, Mani
> Johan
-- மணிவண்ணன் சதாசிவம்
| |