Hi Roderick,
Do you think a good interim solution would be to implement the basic
rumble functionality and to not allow frequency configuration for the
time being? Existing userspace drivers for the most part either just
pick frequencies or make them configurable through a config file.
I assume that if there were to be an FF API overhaul or successor that
we'd still want the driver compatible with the current API. I could
just leave the frequencies as the #define defaults for the moment with
no way for userspace to change them. That would just be a matter of
removing this one patch.
Thanks,
Daniel
On Mon, Mar 11, 2019 at 10:56 PM Roderick Colenbrander
<thunderbird2k@xxxxxxxxx> wrote:
>
> Hi Daniel,
>
> I don't fully understand how rumble / haptics works on the Switch
> controllers, but it sounds like the existing FF API is too limiting.
> I'm not sure if adding new sysfs attributes is the answer to that. I'm
> not sure if we should implement force feedback currently in the Switch
> driver. It is more that once it is there and we change our mind, we
> can't really change it anymore.
>
> There is a lot going around haptics in the industry right now whether
> it is in case of VR controllers and many other areas. I don't have a
> good picture on where things are heading, but it feels like the FF API
> needs to be extended if possible (or if not add a FF2 API).
>
> Thanks,
> Roderick
>
> On Sun, Mar 10, 2019 at 7:04 PM Daniel J. Ogorchock
> <djogorchock@xxxxxxxxx> wrote:
> >
> > This patch adds four sysfs attributes (one for each configurable rumble
> > frequency). This allows userspace software to play different tones on
> > the joy-cons by varying the frequency.
> >
> > Signed-off-by: Daniel J. Ogorchock <djogorchock@xxxxxxxxx>
> > ---
> > drivers/hid/hid-joycon.c | 161 ++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 151 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/hid/hid-joycon.c b/drivers/hid/hid-joycon.c
> > index 10460bac3307..a8ea8b09265d 100644
> > --- a/drivers/hid/hid-joycon.c
> > +++ b/drivers/hid/hid-joycon.c
> > @@ -180,8 +180,6 @@ static const struct joycon_rumble_freq_data joycon_rumble_frequencies[] = {
> > { 0xe801, 0x00, 1124 }, { 0xec01, 0x00, 1149 }, { 0xf001, 0x00, 1174 },
> > { 0xf401, 0x00, 1199 }, { 0xf801, 0x00, 1226 }, { 0xfc01, 0x00, 1253 }
> > };
> > -static const u16 joycon_max_rumble_freq =
> > -joycon_rumble_frequencies[ARRAY_SIZE(joycon_rumble_frequencies) - 1].freq;
> >
> > static const struct joycon_rumble_amp_data joycon_rumble_amplitudes[] = {
> > /* high, low, amp */
> > @@ -353,6 +351,8 @@ struct joycon_ctlr {
> > u16 rumble_lh_freq;
> > u16 rumble_rl_freq;
> > u16 rumble_rh_freq;
> > + u16 rumble_l_amp;
> > + u16 rumble_r_amp;
> > };
> >
> > static int __joycon_hid_send(struct hid_device *hdev, u8 *data, size_t len)
> > @@ -762,11 +762,34 @@ static void joycon_encode_rumble(u8 *data, u16 freq_low, u16 freq_high, u16 amp)
> > data[3] = amp_data.low & 0xFF;
> > }
> >
> > +#define JOYCON_MAX_RUMBLE_HIGH_FREQ ((u16) 1253)
> > +#define JOYCON_MIN_RUMBLE_HIGH_FREQ ((u16) 82)
> > +#define JOYCON_MAX_RUMBLE_LOW_FREQ ((u16) 626)
> > +#define JOYCON_MIN_RUMBLE_LOW_FREQ ((u16) 41)
> > +
> > +static void joycon_clamp_rumble_freqs(struct joycon_ctlr *ctlr)
> > +{
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&ctlr->lock, flags);
> > + ctlr->rumble_ll_freq = clamp(ctlr->rumble_ll_freq,
> > + JOYCON_MIN_RUMBLE_LOW_FREQ,
> > + JOYCON_MAX_RUMBLE_LOW_FREQ);
> > + ctlr->rumble_lh_freq = clamp(ctlr->rumble_lh_freq,
> > + JOYCON_MIN_RUMBLE_HIGH_FREQ,
> > + JOYCON_MAX_RUMBLE_HIGH_FREQ);
> > + ctlr->rumble_rl_freq = clamp(ctlr->rumble_rl_freq,
> > + JOYCON_MIN_RUMBLE_LOW_FREQ,
> > + JOYCON_MAX_RUMBLE_LOW_FREQ);
> > + ctlr->rumble_rh_freq = clamp(ctlr->rumble_rh_freq,
> > + JOYCON_MIN_RUMBLE_HIGH_FREQ,
> > + JOYCON_MAX_RUMBLE_HIGH_FREQ);
> > + spin_unlock_irqrestore(&ctlr->lock, flags);
> > +}
> > +
> > static int joycon_set_rumble(struct joycon_ctlr *ctlr, u16 amp_r, u16 amp_l)
> > {
> > u8 data[8];
> > - u16 freq_low;
> > - u16 freq_high;
> > u16 amp;
> > u16 freq_r_low;
> > u16 freq_r_high;
> > @@ -782,16 +805,14 @@ static int joycon_set_rumble(struct joycon_ctlr *ctlr, u16 amp_r, u16 amp_l)
> > spin_unlock_irqrestore(&ctlr->lock, flags);
> >
> > /* right joy-con */
> > - freq_low = min(freq_r_low, joycon_max_rumble_freq);
> > - freq_high = min(freq_r_high, joycon_max_rumble_freq);
> > amp = amp_r * (u32)joycon_max_rumble_amp / 65535;
> > - joycon_encode_rumble(data + 4, freq_low, freq_high, amp);
> > + ctlr->rumble_r_amp = amp;
> > + joycon_encode_rumble(data + 4, freq_r_low, freq_r_high, amp);
> >
> > /* left joy-con */
> > - freq_low = min(freq_l_low, joycon_max_rumble_freq);
> > - freq_high = min(freq_l_high, joycon_max_rumble_freq);
> > amp = amp_l * (u32)joycon_max_rumble_amp / 65535;
> > - joycon_encode_rumble(data, freq_low, freq_high, amp);
> > + ctlr->rumble_l_amp = amp;
> > + joycon_encode_rumble(data, freq_l_low, freq_l_high, amp);
> >
> > spin_lock_irqsave(&ctlr->lock, flags);
> > memcpy(ctlr->rumble_data, data, sizeof(ctlr->rumble_data));
> > @@ -1241,6 +1262,111 @@ static void joycon_ctlr_destroy(struct joycon_ctlr *ctlr)
> > mutex_destroy(&ctlr->output_mutex);
> > }
> >
> > +static ssize_t joycon_show_freq(struct device *dev,
> > + struct device_attribute *attr, char *buf);
> > +
> > +static ssize_t joycon_store_freq(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count);
> > +
> > +static DEVICE_ATTR(left_high_freq, 0644,
> > + joycon_show_freq, joycon_store_freq);
> > +static DEVICE_ATTR(left_low_freq, 0644,
> > + joycon_show_freq, joycon_store_freq);
> > +static DEVICE_ATTR(right_high_freq, 0644,
> > + joycon_show_freq, joycon_store_freq);
> > +static DEVICE_ATTR(right_low_freq, 0644,
> > + joycon_show_freq, joycon_store_freq);
> > +
> > +static ssize_t joycon_show_freq(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct hid_device *hdev = to_hid_device(dev);
> > + struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
> > + const char *name = attr->attr.name;
> > + u16 val;
> > +
> > + if (!strcmp(name, dev_attr_left_high_freq.attr.name))
> > + val = ctlr->rumble_lh_freq;
> > + else if (!strcmp(name, dev_attr_left_low_freq.attr.name))
> > + val = ctlr->rumble_ll_freq;
> > + else if (!strcmp(name, dev_attr_right_high_freq.attr.name))
> > + val = ctlr->rumble_rh_freq;
> > + else
> > + val = ctlr->rumble_rl_freq;
> > +
> > + return scnprintf(buf, PAGE_SIZE, "%hu\n", val);
> > +}
> > +
> > +static ssize_t joycon_store_freq(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct hid_device *hdev = to_hid_device(dev);
> > + struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
> > + const char *name = attr->attr.name;
> > + int ret;
> > + u16 *val;
> > +
> > + if (!strcmp(name, dev_attr_left_high_freq.attr.name))
> > + val = &ctlr->rumble_lh_freq;
> > + else if (!strcmp(name, dev_attr_left_low_freq.attr.name))
> > + val = &ctlr->rumble_ll_freq;
> > + else if (!strcmp(name, dev_attr_right_high_freq.attr.name))
> > + val = &ctlr->rumble_rh_freq;
> > + else
> > + val = &ctlr->rumble_rl_freq;
> > +
> > + ret = kstrtou16(buf, 10, val);
> > + if (ret >= 0)
> > + ret = count;
> > +
> > + joycon_clamp_rumble_freqs(ctlr);
> > + joycon_set_rumble(ctlr, ctlr->rumble_r_amp, ctlr->rumble_l_amp);
> > +
> > + return ret;
> > +}
> > +
> > +static int joycon_create_sysfs_attribs(struct joycon_ctlr *ctlr)
> > +{
> > + struct device *dev = &ctlr->hdev->dev;
> > + int ret = 0;
> > +
> > + if (ctlr->hdev->product != USB_DEVICE_ID_NINTENDO_JOYCONL) {
> > + ret = device_create_file(dev, &dev_attr_right_high_freq);
> > + if (!ret) {
> > + ret = device_create_file(dev, &dev_attr_right_low_freq);
> > + if (ret)
> > + device_remove_file(dev,
> > + &dev_attr_right_high_freq);
> > + }
> > + }
> > + if (ctlr->hdev->product != USB_DEVICE_ID_NINTENDO_JOYCONR && !ret) {
> > + ret = device_create_file(dev, &dev_attr_left_high_freq);
> > + if (!ret) {
> > + ret = device_create_file(dev, &dev_attr_left_low_freq);
> > + if (ret)
> > + device_remove_file(dev,
> > + &dev_attr_left_high_freq);
> > + }
> > + }
> > + return ret;
> > +}
> > +
> > +static void joycon_remove_sysfs_attribs(struct joycon_ctlr *ctlr)
> > +{
> > + struct device *dev = &ctlr->hdev->dev;
> > +
> > + if (ctlr->hdev->product != USB_DEVICE_ID_NINTENDO_JOYCONL) {
> > + device_remove_file(dev, &dev_attr_right_high_freq);
> > + device_remove_file(dev, &dev_attr_right_low_freq);
> > + }
> > + if (ctlr->hdev->product != USB_DEVICE_ID_NINTENDO_JOYCONR) {
> > + device_remove_file(dev, &dev_attr_left_high_freq);
> > + device_remove_file(dev, &dev_attr_left_low_freq);
> > + }
> > +}
> > +
> > static int joycon_hid_probe(struct hid_device *hdev,
> > const struct hid_device_id *id)
> > {
> > @@ -1330,6 +1456,16 @@ static int joycon_hid_probe(struct hid_device *hdev,
> > goto err_close;
> > }
> >
> > + /* Create the sysfs entries for configuring rumble frequencies */
> > + if (IS_ENABLED(CONFIG_JOYCON_FF)) {
> > + ret = joycon_create_sysfs_attribs(ctlr);
> > + if (ret) {
> > + hid_err(hdev, "Failed to create sysfs attrs; ret=%d\n",
> > + ret);
> > + goto err_close;
> > + }
> > + }
> > +
> > /* Initialize the leds */
> > ret = joycon_leds_create(ctlr);
> > if (ret) {
> > @@ -1368,6 +1504,11 @@ static void joycon_hid_remove(struct hid_device *hdev)
> > hid_dbg(hdev, "remove\n");
> > hid_hw_close(hdev);
> > hid_hw_stop(hdev);
> > +
> > + /* Remove the sysfs entries if rumble is enabled */
> > + if (IS_ENABLED(CONFIG_JOYCON_FF))
> > + joycon_remove_sysfs_attribs(ctlr);
> > +
> > joycon_ctlr_destroy(ctlr);
> > }
> >
> > --
> > 2.21.0
> >
--
Daniel Ogorchock
