Re: Revisiting ITE8708 on ASUS PN50 uses a 16 byte io region

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks for your response Sean

On Thu, Mar 18, 2021 at 11:48 AM Sean Young <sean@xxxxxxxx> wrote:
>
> Hi Nikolaos,
>
> On Wed, Mar 17, 2021 at 04:41:15PM +0200, Nikolaos Beredimas wrote:
> > Hi,
> > There was a thread on this list last September
> > https://www.spinics.net/lists/linux-media/msg177724.html
> > about the IR module on the ASUS PN50.
> >
> > Even though that discussion never fully resolved,
> > it did contain the solution to get the IR working on the PN50.
> > I have documented this at
> > https://forum.libreelec.tv/thread/23145-asus-pn50-challenge/?postID=152207#post152207
> >
> > So, what I had to do is edit a single line of drivers/media/rc/ite-cir.h
> > and change IT8708_IOREG_LENGTH 0x08 to IT8708_IOREG_LENGTH 0x10
> > and the IR module is now recognized and working
> >
> > How do I go about submitting this as a patch?
> > I am a little overwhelmed honestly.
> > Do I follow https://www.linuxtv.org/wiki/index.php/Development:_How_to_submit_patches
> > ?
> > And which git tree?
>
> Thanks for fixing this. The patch should be a diff against
> https://git.linuxtv.org/media_tree.git/
>
> This is the guide for submitting patches:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>
>
> >
> > --- a/drivers/media/rc/ite-cir.h
> > +++ b/drivers/media/rc/ite-cir.h
> > @@ -406,7 +406,7 @@
> >  #define IT8708_C0WCR 0x06 /* wakeup code read/write register */
> >  #define IT8708_C0WPS 0x07 /* wakeup power control/status register */
> >
> > -#define IT8708_IOREG_LENGTH 0x08 /* length of register file */
> > +#define IT8708_IOREG_LENGTH 0x10 /* length of register file */
>
> I don't think this is correct though. There are other devices that have
> length of 8; I think the correct solution.
>
> I think:
>
>         if (!pnp_port_valid(pdev, io_rsrc_no) ||
>             pnp_port_len(pdev, io_rsrc_no) != dev_desc->io_region_size) {
>                 dev_err(&pdev->dev, "IR PNP Port not valid!\n");
>                 goto exit_free_dev_rdev;
>         }
>
>
> should be changed to:
>
>         if (!pnp_port_valid(pdev, io_rsrc_no) ||
>             pnp_port_len(pdev, io_rsrc_no) < dev_desc->io_region_size) {
>                 dev_err(&pdev->dev, "IR PNP Port not valid!\n");
>                 goto exit_free_dev_rdev;
>         }
>
> Thanks
>

Indeed. This was also pointed to me by another user at the LibreELEC forum where
I documented my initial "fix" (which although fixed the problem for my
specific hw,
would probably bork every other ITE8708 in existence).
I have implemented the proper fix per your suggestion, and have tested
it against
kernel version 5.10.21 which is the one used by the latest LibreELEC beta.
I think I have reached the limit of my abilities here.
Following is my first attempt at submitting this as a patch,
against the master branch of git://linuxtv.org/media_tree.git
NB
---------------

Accept larger io_region_size for ITE8708 in ite-cir.c
ITE8708 on ASUS PN50 uses a 16 byte io region. This issue was
first identified by Michael Zimmermann in
https://www.spinics.net/lists/linux-media/msg177724.html
and prevents the driver from loading on the ASUS PN50, throwing
the 'IR PNP Port not valid!' error.
Current understanding is that the issue lies on erroneous DSDT
ACPI tables on ASUS part, that advertise the register length as
16 bytes, and not 8 bytes as defined in ite-cir.h
(IT8708_IOREG_LENGTH).
In any case, this patch changes the strict not-equal check in the
ite_probe function to a less-than check, allowing the driver
to load despite the fact that the DSDT claims a larger register.

Signed-off-by: Nikolaos Beredimas <beredim@xxxxxxxxx>
---
 drivers/media/rc/ite-cir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
index 9388774df9d7..5bc23e8c6d91 100644
--- a/drivers/media/rc/ite-cir.c
+++ b/drivers/media/rc/ite-cir.c
@@ -1333,7 +1333,7 @@ static int ite_probe(struct pnp_dev *pdev, const
struct pnp_device_id

/* validate pnp resources */
if (!pnp_port_valid(pdev, io_rsrc_no) ||
-     pnp_port_len(pdev, io_rsrc_no) != dev_desc->io_region_size) {
+     pnp_port_len(pdev, io_rsrc_no) < dev_desc->io_region_size) {
dev_err(&pdev->dev, "IR PNP Port not valid!\n");
goto exit_free_dev_rdev;
}
--



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux