Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20191126212753.18455-1-pali.rohar@gmail.com> <CABBYNZ+X1xThm9rrcj7ny9f9r0sVs-fTR2JroQdRgZTRWLcN6g@mail.gmail.com>
 <20191127160720.aleatv6l4swopg57@pali>
In-Reply-To: <20191127160720.aleatv6l4swopg57@pali>
From:   Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Date:   Wed, 27 Nov 2019 09:21:24 -0800
Message-ID: <CANFp7mW47eJ6dquzLYX-uKoAO=Qc=ObgmSWQ5U0Xs7ThEV0Urg@mail.gmail.com>
Subject: Re: [PATCH] avctp: Set more descriptive name for uinput device
To:     =?UTF-8?Q?Pali_Roh=C3=A1r?= <pali.rohar@gmail.com>
Cc:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

On Wed, Nov 27, 2019 at 8:07 AM Pali Roh=C3=A1r <pali.rohar@gmail.com> wrot=
e:
>
> Hi!
>
> On Wednesday 27 November 2019 17:50:40 Luiz Augusto von Dentz wrote:
> > Hi Pali,
> >
> > On Tue, Nov 26, 2019 at 11:31 PM Pali Roh=C3=A1r <pali.rohar@gmail.com>=
 wrote:
> > >
> > > ---
> > >  profiles/audio/avctp.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> > > index 70a3e40b2..ae53587ad 100644
> > > --- a/profiles/audio/avctp.c
> > > +++ b/profiles/audio/avctp.c
> > > @@ -1181,8 +1181,7 @@ static int uinput_create(char *name)
> > >         }
> > >
> > >         memset(&dev, 0, sizeof(dev));
> > > -       if (name)
> > > -               strncpy(dev.name, name, UINPUT_MAX_NAME_SIZE - 1);
> > > +       snprintf(dev.name, sizeof(dev.name), "Bluetooth Audio/Video R=
emote Control%s%s", name ? " " : "", name ? name : "");
> > >
> > >         dev.id.bustype =3D BUS_BLUETOOTH;
> > >         dev.id.vendor  =3D 0x0000;
> > > --
> > > 2.11.0
> >
> > It is already setting a bustype to BUS_BLUETOOTH why do you need to
> > make it more specific,
>
> Because more tools shows only device name. Also in kernel dmesg is only
> device name. And MAC address is really something which does not say
> anything about device...
>
> For this reason also older input devices (implemented in kernel) exports
> more descriptive names, e.g. "AT Translated Set 2 keyboard" or "ETPS/2
> Elantech TrackPoint" or "Integrated IR Camera".
>
> > btw it needs to be limited to UINPUT_MAX_NAME_SIZE.
>
> Is not this implicitly defined by sizeof()?
>
> > Id say if we want to make it declare the connection type
>
> Yes, connection type would be really useful.
>
> Now I'm playing with exporting "button press even" from HSP profile via
> uinput device too and therefore connection type should be there.
>
> > that probably something that needs to be encoded in
> > the bus itself, like BUS_BLUETOOTH_AVCTP, BUS_BLUETOOTH_HOG, etc.
>
> But bus type is kernel API and there is no such AVCTP or HOG. So we need
> to stick with BUS_BLUETOOH.
>
> Other kernel devices also put protocol information into name (e.g.
> touchpad / trackpoints), so it is ideal place also for bluetooth
> devices.
>
> And probably most userspace devices are mapped to BUS_VIRTUAL. There
> are no subtypes.
>
> --
> Pali Roh=C3=A1r
> pali.rohar@gmail.com

Wouldn't it be better to use the device name for uinput rather than the add=
ress?

In `init_uinput` (avctp.c), uinput_create is called with
device_get_address instead of device_get_name. It would probably be
better to use the device name for the uinput and set the device
address as the `uniq` attribute (as it is done for /dev/uhid for HID
devices).

So in the following, replace ATTR{name} with something like "Some
Company Headphones", ATTR{uniq} =3D "33:33:33:ff:ff:ff" and ATTR{phys}
with the controller's address.

$ udevadm info -a -p /sys/devices/virtual/input/input21/
...
  looking at device '/devices/virtual/input/input21':
    KERNEL=3D=3D"input21"
    SUBSYSTEM=3D=3D"input"
    DRIVER=3D=3D""
    ATTR{inhibited}=3D=3D"0"
    ATTR{name}=3D=3D"33:33:33:FF:FF:FF"
    ATTR{phys}=3D=3D""
    ATTR{properties}=3D=3D"0"
    ATTR{uniq}=3D=3D""

This is what uhid looks like in comparison:

$ udevadm info -a -p /sys/class/misc/uhid/0005\:046D\:B33B.0002/input/input=
18/

  looking at device
'/devices/virtual/misc/uhid/0005:046D:B33B.0002/input/input18':
    KERNEL=3D=3D"input17"
    SUBSYSTEM=3D=3D"input"
    DRIVER=3D=3D""
    ATTR{inhibited}=3D=3D"0"
    ATTR{name}=3D=3D"Keyboard K780 Keyboard"
    ATTR{phys}=3D=3D"ab:cd:ef:12:34:56"
    ATTR{properties}=3D=3D"0"
    ATTR{uniq}=3D=3D"33:33:33:33:ff:ff"


Abhishek