On Thu, 10 Nov 2022, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Tue, Nov 08, 2022 at 05:18:26PM +0200, Imre Deak wrote:
>> +static void
>> +main_link_aux_power_domain_put(struct intel_digital_port *dig_port,
>> + const struct intel_crtc_state *crtc_state)
>> +{
>> + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
>> + intel_wakeref_t wf = fetch_and_zero(&dig_port->aux_wakeref);
>
> Please don't call functions with side effects in variable
> declaration blocks. Far too easy to miss them and then you end up
> scratching your head for a day or two debugging the wrong thing.
Side note, if I got to decide, I'd probably nuke "fetch_and_zero" out of
existence.
It's not that I don't find the concept useful, it's the naming that
gives the impression of atomicity that the macro utterly lacks. I find
that dangerous.
It's a helper that feels like should be part of a core kernel header
(and you might mistakenly think it already is!) but I doubt would ever
pass muster because of the above.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
