Re: help needed in this part of code regarding FF in hid-sony.c

Hi Fei,

I'm not sure why you are trying to make such change kernel side, but
of course nothing is stopping you for personal use.

Kernel development is not easy and it is very easy to crash your
system. I can't say what is wrong here, but among the most common
issues are null pointer dereferences. If lucky the error is caught in
your kernel log, which should give you some clues on where the error
is coming from.

Also in your changes, I see you added some sleeps. Sleeping in the
kernel is taboo in a lot of places and depending on where is done can
also be a thing leading to issues as well. Depending on where you
tried to initiate some of this new code, the driver might not have
completed fully initializing, so some variables might not be set or
buffers might not be fully initialized.

Thanks,
Roderick

On Mon, Aug 30, 2021 at 12:11 AM fei fei [off-list ref] wrote:

4th attempt......

Hello to Mr. Roderick or to whom it may concren,

I am fei, a novice self-studied programming enthusiast, I need help
with hid-sony.c, but there is no relevant maintainer found in the
MAINTAINERS list, the closest is you, Mr. Roderick as the maintainer
of hid-playstation.c, so I just try my luck here, hope you don't mind.

The scenario as follow:

I have added a feature to make initial rumble vibrates when devices
are connected, code as follow (sony_play_effect() as reference) :

add delay.h

-----------------------------
#include <linux/delay.h>
-----------------------------





define a global variable "init_rumble"

------------------------------
......#define SONY_FF_SUPPORT (SIXAXIS_CONTROLLER | DUALSHOCK4_CONTROLLER)

bool init_rumble;
#define SONY_BT_DEVICE......
------------------------------




declare "init_rumble = true;" in sony_probe()


actual part

------------------------------
static int sony_init_ff_play(struct input_dev *dev)
{
    struct hid_device *hid = input_get_drvdata(dev);
    struct sony_sc *sc = hid_get_drvdata(hid);

    sc->left = 255;
    sc->right = 255;

/*needed for non bt connection or else won't work, reason unknown*/
    if (!(sc->quirks & SONY_BT_DEVICE))
          sony_schedule_work(sc, SONY_WORKER_STATE);

    /*length-ing magnitude above*/
    msleep(350);

    sc->left = 0;
    sc->right = 0;
    sony_schedule_work(sc, SONY_WORKER_STATE);

    init_rumble = false;

    return 0;
}
-------------------------------




and called from sony_init_ff()

--------------------------------
......input_set_capability(input_dev, EV_FF, FF_RUMBLE);

if (init_rumble == true) {
    sony_init_ff_play(input_dev);
}

return input_ff_create_memless(input_dev......
--------------------------------




it works flawlessly without any error. Since it's identical to
sony_play_effect(), so i just tried to merge them together into
sony_play_effect() to reduce redundancy as follow:

------------------------------------
static int sony_play_effect(struct input_dev *dev, void *data,
   struct ff_effect *effect)
{
    struct hid_device *hid = input_get_drvdata(dev);
    struct sony_sc *sc = hid_get_drvdata(hid);

    if (effect->type != FF_RUMBLE)
        return 0;

    if (init_rumble == true)
    {
          sc->left = 255;
          sc->right = 255;

          if (!(sc->quirks & SONY_BT_DEVICE))
              sony_schedule_work(sc, SONY_WORKER_STATE);

          msleep(400);

          sc->left = 0;
          sc->right = 0;
          sony_schedule_work(sc, SONY_WORKER_STATE);

          init_rumble = false;
          return 0;
    }

    sc->left = effect->u.rumble.strong_magnitude / 256;
    sc->right = effect->u.rumble.weak_magnitude / 256;

    sony_schedule_work(sc, SONY_WORKER_STATE);

    return 0;
}
-----------------------------------------------




called it from sony_init_ff()

----------------------------------------------
 ......input_set_capability(input_dev, EV_FF, FF_RUMBLE);

   if (init_rumble == true) {
       sony_play_effect(input_dev, NULL, NULL);
   }

return input_ff_create_memless(input_dev......
------------------------------------------------




but end up whole system being freezed up, what could possibly going
wrong here? Thx in advance.

Regards,
fei