From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751563AbcIBFZV (ORCPT ); Fri, 2 Sep 2016 01:25:21 -0400 Received: from mailout3.samsung.com ([203.254.224.33]:34710 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750979AbcIBFZR (ORCPT ); Fri, 2 Sep 2016 01:25:17 -0400 X-AuditID: cbfee68e-f79cb6d000006cfe-79-57c90d3ab4dd Date: Fri, 02 Sep 2016 14:25:14 +0900 From: Andi Shyti To: Sean Young Cc: Mauro Carvalho Chehab , Rob Herring , Mark Rutland , linux-media@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Andi Shyti Subject: Re: [PATCH v2 1/7] [media] rc-main: assign driver type during allocation Message-id: <20160902052514.fmjk4mrhblztthzb@samsunx.samsung> References: <20160901171629.15422-1-andi.shyti@samsung.com> <20160901171629.15422-2-andi.shyti@samsung.com> <20160901212351.GB22198@gofer.mess.org> MIME-version: 1.0 Content-type: text/plain; charset=us-ascii Content-disposition: inline In-reply-to: <20160901212351.GB22198@gofer.mess.org> User-Agent: NeoMutt/ (1.7.0) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrLIsWRmVeSWpSXmKPExsWyRsSkQNea92S4wbuzTBaLfzxnsph/5Byr xeVdc9gsejZsZbVYev0ik8XqZxUWrXuPsFssfPqdxYHDY828NYwe15d8YvbYtKqTzWPJG2uP Lf132T0+b5ILYIvisklJzcksSy3St0vgyvi7ditjwUvBimsf+RsY7/N0MXJySAiYSJy9dpEd whaTuHBvPVsXIxeHkMAKRolJuy8ywRRNn9wDlVjKKPHifRMThPORUWLd6/eMIFUsAqoSr7/M YQWx2QQ0JZpu/2ADsUUE5CS+bWthBWlgFvjGKPG/ZztYQlggWGL2tllgu3kFbCVu3j7ECDH1 NaPE1EerWCESghI/Jt9jAbGZBbQk1u88zgRhS0s8+jsDrJkT6L4Vq1rArhAVkJHo3LYO7DwJ gXvsEn1tD1ggzhOQ+Db5EJDNAZSQldh0gBniN0mJgytusExgFJuFZN0sJOtmIVm3gJF5FaNo akFyQXFSepGRXnFibnFpXrpecn7uJkZgLJ7+96xvB+PNA9aHGAU4GJV4eC2WnwgXYk0sK67M PcRoCnTFRGYp0eR8YMTnlcQbGpsZWZiamBobmVuaKYnzJkj9DBYSSE8sSc1OTS1ILYovKs1J LT7EyMTBKdXAmDj/Xzi3R7D+UguL0/L6eezG7S9fCzV+Fb59tO6+7ek/s97H2axew9pj+XN2 toL5lsnGaSYLmyZWXn2xrcGz1V0gWEzP3fa+FO8nfv6H3yJKzi08zTCzlO2a2j7Dp50+19Jd rOqWNR+qrNJeNkPwy5Ujc+vqn3zKP70/ROzJl3vPQ+Z+ieIuU2Ipzkg01GIuKk4EAOwafs3A AgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrFIsWRmVeSWpSXmKPExsVy+t9jAV0r3pPhBjMfW1os/vGcyWL+kXOs Fpd3zWGz6NmwldVi6fWLTBarn1VYtO49wm6x8Ol3FgcOjzXz1jB6XF/yidlj06pONo8lb6w9 tvTfZff4vEkugC2qgdEmIzUxJbVIITUvOT8lMy/dVsk7ON453tTMwFDX0NLCXEkhLzE31VbJ xSdA1y0zB+geJYWyxJxSoFBAYnGxkr4dpgmhIW66FjCNEbq+IUFwPUYGaCBhDWPG37VbGQte ClZc+8jfwHifp4uRk0NCwERi+uQeNghbTOLCvfVANheHkMBSRokX75uYIJyPjBLrXr9nBKli EVCVeP1lDiuIzSagKdF0+wdYt4iAnMS3bS2sIA3MAt8YJf73bAdLCAsES8zeNosdxOYVsJW4 efsQI8TU14wSUx+tYoVICEr8mHyPBcRmFtCSWL/zOBOELS3x6O8MsGZOoFtXrGoBu0JUQEai c9s6pgmMArOQtM9C0j4LSfsCRuZVjBKpBckFxUnpuYZ5qeV6xYm5xaV56XrJ+bmbGMHx/kxq B+PBXe6HGAU4GJV4eBtmnwgXYk0sK67MPcQowcGsJMKbzXIyXIg3JbGyKrUoP76oNCe1+BCj KTBIJjJLiSbnA1NRXkm8obGJmZGlkbmhhZGxuZI47+P/68KEBNITS1KzU1MLUotg+pg4OKUa GNsjLxlslrl6afepR699Ozc4GHl2qq6JLpj24uBm9WV/O3y8rPhqGFiTPy0/wHFn1wy9wgsc N6adsO0tWXnGWdVm+vl3eivuPYr8WmhU9s/s+33dbBNxYWZzx47Mk5b3atuYJ87yVLhb3Jdk cv+s5ZI7P88+f7QmKMOFveZglfqmqfwT1dP4niqxFGckGmoxFxUnAgDt0wqcDQMAAA== DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Sean, > > ir = kzalloc(sizeof(*ir), GFP_KERNEL); > > - dev = rc_allocate_device(); > > + dev = rc_allocate_device(RC_DRIVER_IR_RAW); > > if (!ir || !dev) > > goto err_out_free; > > > > If ir->sampling = 0 then it should be RC_DRIVER_SCANCODE. > > > > @@ -481,7 +481,6 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci) > > dev->scancode_mask = hardware_mask; > > > > if (ir->sampling) { > > - dev->driver_type = RC_DRIVER_IR_RAW; > > dev->timeout = 10 * 1000 * 1000; /* 10 ms */ > > } else { > > dev->driver_type = RC_DRIVER_SCANCODE; > > That assignment shouldn't really be there any more. I think this doesn't change the driver's behavior, because I either do like: - dev = rc_allocate_device(); + dev = rc_allocate_device(RC_DRIVER_SCANCODE); [ ... ] if (ir->sampling) { dev->driver_type = RC_DRIVER_IR_RAW; dev->timeout = 10 * 1000 * 1000; /* 10 ms */ } else { - dev->driver_type = RC_DRIVER_SCANCODE; Or I would need to do aftr the long switch...case statement + if (ir->sampling) { + dev = rc_allocate_device(RC_DRIVER_IR_RAW); + ... + } else { + dev = rc_allocate_device(RC_DRIVER_SCANCODE); + ... I prefered the first way because it doesn't alter much the driver. > > ir = kzalloc(sizeof(*ir), GFP_KERNEL); > > - rc = rc_allocate_device(); > > + rc = rc_allocate_device(RC_DRIVER_SCANCODE); > > if (!ir || !rc) { > > err = -ENOMEM; > > goto err_out_free; > > This is not correct, I'm afraid. If you look at the code you can see that > if raw_decode is true, then it should be RC_DRIVER_IR_RAW. Same here, the driver doesn't change the behavior. raw_decode can be both 'true' or 'false' it's set as default RC_DRIVER_SCANCODE and depending on value of raw_decode it's chaged to RC_DRIVER_IR_RAW. also in this case I can do + if (raw_decode) { + rc = rc_allocate_device(RC_DRIVER_IR_RAW); + ... + } else { + rc = rc_allocate_device(RC_DRIVER_SCANCODE); + ... but also in this case my original approach doesn't add much changes. Thanks, Andi