Hi
2021. január 8., péntek 7:06 keltezéssel, Roderick Colenbrander írta:
> [...]
> > > +static int dualsense_get_calibration_data(struct dualsense *ds)
> > > +{
> > > + short gyro_pitch_bias, gyro_pitch_plus, gyro_pitch_minus;
> > > + short gyro_yaw_bias, gyro_yaw_plus, gyro_yaw_minus;
> > > + short gyro_roll_bias, gyro_roll_plus, gyro_roll_minus;
> > > + short gyro_speed_plus, gyro_speed_minus;
> > > + short acc_x_plus, acc_x_minus;
> > > + short acc_y_plus, acc_y_minus;
> > > + short acc_z_plus, acc_z_minus;
> > > + int speed_2x;
> > > + int range_2g;
> > > + int ret = 0;
> > > + uint8_t *buf;
> > > +
> > > + buf = kzalloc(DS_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL);
> > > + if (!buf)
> > > + return -ENOMEM;
> > > +
> > > + ret = hid_hw_raw_request(ds->base.hdev, DS_FEATURE_REPORT_CALIBRATION, buf,
> > > + DS_FEATURE_REPORT_CALIBRATION_SIZE, HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> >
> > I think it would be better if lines were aligned. I have missed this in other patches,
> > so if you decide to make this change, please do it everywhere.
>
> What do you mean with "if lines were aligned"? You mean aligning the
> DS_FEATURE.. part with ds->base.hdev?
Yes, exactly.
>
> I'm almost tempted in the future (as part of a future patch series) to
> perhaps have a ps_device_get_feature_report or something like that as
> there is the same code in multiple places. It can do some nicer
> checking as well (including to see if the first byte is the report ID
> number, which is guaranteed for DualSense). I think it is a bit much
> to add now, but probably in the future also when I add DualShock 4 in
> here.
I think it's a good idea to add such a function sometime.
>
> >
> > > + if (ret < 0)
> > > + goto err_free;
> > > + else if (ret != DS_FEATURE_REPORT_CALIBRATION_SIZE) {
> >
> > As per coding style[1], please either use {} for all branches, or just drop the
> > `else` and maybe add a new line:
> >
> > ```
> > if (ret < 0)
> > goto ...
> >
> > if (ret != ...) {
> > ...
> > }
> > ```
> >
> >
> > > + hid_err(ds->base.hdev, "failed to retrieve DualSense calibration info\n");
> >
> > I think this message could be improved to better pinpoint the exact problem
> > that triggered it.
> >
> >
> > > + ret = -EINVAL;
> > > + goto err_free;
> > > + }
> [...]
Regards,
Barnabás Pőcze
