Message ID | 1454327094-7848-1-git-send-email-majun258@huawei.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Mon, Feb 01, 2016 at 07:44:54PM +0800, MaJun wrote: > From: Ma Jun <majun258@huawei.com> > > For mbigen module, there is a special case that more than one mbigen > device nodes use the same reg definition in DTS when these devices > exist in the same mbigen hardware module. > > mbigen_dev1:intc_dev1 { > ... > reg = <0x0 0xc0080000 0x0 0x10000>; > ... > }; > > mbigen_dev2:intc_dev2 { > ... > reg = <0x0 0xc0080000 0x0 0x10000>; > ... > }; This doesn't sound right. If they exist in the same place, and have the same reg, they _are_ the same device. You'll need to explain this better. > On this case, devm_ioremap_resource() returns fail with info > "can't request region for resource" because of memory region check. > > Because we only need to program 1 into corresponding bit into status > register of mbigen to clear the interrupt status during runtime, > I think we can replace devm_ioremap_resource() by devm_ioremap(). > > Signed-off-by: Ma Jun <majun258@huawei.com> > --- > drivers/irqchip/irq-mbigen.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c > index 4dd3eb8..b19e528 100644 > --- a/drivers/irqchip/irq-mbigen.c > +++ b/drivers/irqchip/irq-mbigen.c > @@ -250,7 +250,7 @@ static int mbigen_device_probe(struct platform_device *pdev) > mgn_chip->pdev = pdev; > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - mgn_chip->base = devm_ioremap_resource(&pdev->dev, res); > + mgn_chip->base = devm_ioremap(&pdev->dev, res->start, resource_size(res)); These two behaving differently doesn't seem correct either... Mark. > if (IS_ERR(mgn_chip->base)) > return PTR_ERR(mgn_chip->base); > > -- > 1.7.1 > >
? 2016/2/1 19:57, Mark Rutland ??: > On Mon, Feb 01, 2016 at 07:44:54PM +0800, MaJun wrote: >> From: Ma Jun <majun258@huawei.com> >> >> For mbigen module, there is a special case that more than one mbigen >> device nodes use the same reg definition in DTS when these devices >> exist in the same mbigen hardware module. >> >> mbigen_dev1:intc_dev1 { >> ... >> reg = <0x0 0xc0080000 0x0 0x10000>; >> ... >> }; >> >> mbigen_dev2:intc_dev2 { >> ... >> reg = <0x0 0xc0080000 0x0 0x10000>; >> ... >> }; > > This doesn't sound right. If they exist in the same place, and have the > same reg, they _are_ the same device. > > You'll need to explain this better. > For a mbigen hardware module which connecte with several devices, because these devices has different device ID, I need to define a device node for each device in DTS file. mbigen | ------------------ | | | dev1 dev2 dev3 Because of register layout,the registers related with these devices are put together, I can't split them clearly. So, I only make these devices which connect with same mbigen hardware module usethe same address. >> On this case, devm_ioremap_resource() returns fail with info >> "can't request region for resource" because of memory region check. >> >> Because we only need to program 1 into corresponding bit into status >> register of mbigen to clear the interrupt status during runtime, >> I think we can replace devm_ioremap_resource() by devm_ioremap(). >> >> Signed-off-by: Ma Jun <majun258@huawei.com> >> --- >> drivers/irqchip/irq-mbigen.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c >> index 4dd3eb8..b19e528 100644 >> --- a/drivers/irqchip/irq-mbigen.c >> +++ b/drivers/irqchip/irq-mbigen.c >> @@ -250,7 +250,7 @@ static int mbigen_device_probe(struct platform_device *pdev) >> mgn_chip->pdev = pdev; >> >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> - mgn_chip->base = devm_ioremap_resource(&pdev->dev, res); >> + mgn_chip->base = devm_ioremap(&pdev->dev, res->start, resource_size(res)); > > These two behaving differently doesn't seem correct either... The problem is caused by memory region check fail because of duplicated address in devm_ioremap_resource(). For mbigen special case, there is no necessary to do this check. Thanks Ma Jun > > Mark. > >> if (IS_ERR(mgn_chip->base)) >> return PTR_ERR(mgn_chip->base); >> >> -- >> 1.7.1 >> >> > > . >
On Mon, Feb 01, 2016 at 09:06:38PM +0800, majun (F) wrote: > > > ? 2016/2/1 19:57, Mark Rutland ??: > > On Mon, Feb 01, 2016 at 07:44:54PM +0800, MaJun wrote: > >> From: Ma Jun <majun258@huawei.com> > >> > >> For mbigen module, there is a special case that more than one mbigen > >> device nodes use the same reg definition in DTS when these devices > >> exist in the same mbigen hardware module. > >> > >> mbigen_dev1:intc_dev1 { > >> ... > >> reg = <0x0 0xc0080000 0x0 0x10000>; > >> ... > >> }; > >> > >> mbigen_dev2:intc_dev2 { > >> ... > >> reg = <0x0 0xc0080000 0x0 0x10000>; > >> ... > >> }; > > > > This doesn't sound right. If they exist in the same place, and have the > > same reg, they _are_ the same device. > > > > You'll need to explain this better. > > > > For a mbigen hardware module which connecte with several devices, > because these devices has different device ID, > I need to define a device node for each device in DTS file. > > mbigen > | > ------------------ > | | | > dev1 dev2 dev3 > > Because of register layout,the registers related with these devices > are put together, I can't split them clearly. > So, I only make these devices which connect with > same mbigen hardware module usethe same address. This sounds like we've either mis-described the mbigen in the binding, or we're mis-describing the relationship with the devices. It should not be necessary to describe the same set of registers repeatedly. > >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > >> - mgn_chip->base = devm_ioremap_resource(&pdev->dev, res); > >> + mgn_chip->base = devm_ioremap(&pdev->dev, res->start, resource_size(res)); > > > > These two behaving differently doesn't seem correct either... > > The problem is caused by memory region check fail because of > duplicated address in devm_ioremap_resource(). The check is sensible. Using devm_ioremap rather than devm_ioremap_resource to avoid the check is not the correct solution. > For mbigen special case, there is no necessary to do this check. I'm not sure I fully agree. Regardless, this is not the right solution; it is fragile at best. Thanks, Mark.
Hi Mark: ? 2016/2/1 21:25, Mark Rutland ??: > On Mon, Feb 01, 2016 at 09:06:38PM +0800, majun (F) wrote: >> >> >> ? 2016/2/1 19:57, Mark Rutland ??: >>> On Mon, Feb 01, 2016 at 07:44:54PM +0800, MaJun wrote: >>>> From: Ma Jun <majun258@huawei.com> >>>> >>>> For mbigen module, there is a special case that more than one mbigen >>>> device nodes use the same reg definition in DTS when these devices >>>> exist in the same mbigen hardware module. >>>> >>>> mbigen_dev1:intc_dev1 { >>>> ... >>>> reg = <0x0 0xc0080000 0x0 0x10000>; >>>> ... >>>> }; >>>> >>>> mbigen_dev2:intc_dev2 { >>>> ... >>>> reg = <0x0 0xc0080000 0x0 0x10000>; >>>> ... >>>> }; >>> >>> This doesn't sound right. If they exist in the same place, and have the >>> same reg, they _are_ the same device. >>> >>> You'll need to explain this better. >>> >> >> For a mbigen hardware module which connecte with several devices, >> because these devices has different device ID, >> I need to define a device node for each device in DTS file. >> >> mbigen >> | >> ------------------ >> | | | >> dev1 dev2 dev3 >> >> Because of register layout,the registers related with these devices >> are put together, I can't split them clearly. >> So, I only make these devices which connect with >> same mbigen hardware module usethe same address. > > This sounds like we've either mis-described the mbigen in the binding, > or we're mis-describing the relationship with the devices. > Sorry, I didn't express myself clearly. For example, a mbigen hardware module can support several peripheral devices with different device ID. |-----------------------|- | mbigen | |-----------------------|- | | | dev1 dev2 dev3 To simplify the mbigen drvier, I didn't use the whole mbigen module as a mbgien device, but use the register collections(vector, trigger type,status etc.) corresponding to a peripheral device as a mbigen device. So, mbigen device is a logical conception or logical device. Now, a mbigen hardware module contains several logical mbigen device. ------------------------------- |mgn_dev1 mgn_dev2 mgn_dev3 | |-----------------------------| | | | dev1 dev2 dev3 So,mgn_dev1, mgn_dev2 and mgn_dev3 exist in same mbigen hardware module, and,they use the same reg address region,that is adress of mbigen hardware module. For this case, I think the question is can we map an reg address region more than one time? Thanks! > It should not be necessary to describe the same set of registers > repeatedly. > >>>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>> - mgn_chip->base = devm_ioremap_resource(&pdev->dev, res); >>>> + mgn_chip->base = devm_ioremap(&pdev->dev, res->start, resource_size(res)); >>> >>> These two behaving differently doesn't seem correct either... >> >> The problem is caused by memory region check fail because of >> duplicated address in devm_ioremap_resource(). > > The check is sensible. Using devm_ioremap rather than > devm_ioremap_resource to avoid the check is not the correct solution. > >> For mbigen special case, there is no necessary to do this check. > > I'm not sure I fully agree. > > Regardless, this is not the right solution; it is fragile at best. > > Thanks, > Mark. > > . >
On Tue, Feb 02, 2016 at 06:25:53PM +0800, majun (F) wrote: > Hi Mark: > > ? 2016/2/1 21:25, Mark Rutland ??: > > On Mon, Feb 01, 2016 at 09:06:38PM +0800, majun (F) wrote: > >> > >> > >> ? 2016/2/1 19:57, Mark Rutland ??: > >>> On Mon, Feb 01, 2016 at 07:44:54PM +0800, MaJun wrote: > >>>> From: Ma Jun <majun258@huawei.com> > >>>> > >>>> For mbigen module, there is a special case that more than one mbigen > >>>> device nodes use the same reg definition in DTS when these devices > >>>> exist in the same mbigen hardware module. > >>>> > >>>> mbigen_dev1:intc_dev1 { > >>>> ... > >>>> reg = <0x0 0xc0080000 0x0 0x10000>; > >>>> ... > >>>> }; > >>>> > >>>> mbigen_dev2:intc_dev2 { > >>>> ... > >>>> reg = <0x0 0xc0080000 0x0 0x10000>; > >>>> ... > >>>> }; > >>> > >>> This doesn't sound right. If they exist in the same place, and have the > >>> same reg, they _are_ the same device. > >>> > >>> You'll need to explain this better. > >>> > >> > >> For a mbigen hardware module which connecte with several devices, > >> because these devices has different device ID, > >> I need to define a device node for each device in DTS file. > >> > >> mbigen > >> | > >> ------------------ > >> | | | > >> dev1 dev2 dev3 > >> > >> Because of register layout,the registers related with these devices > >> are put together, I can't split them clearly. > >> So, I only make these devices which connect with > >> same mbigen hardware module usethe same address. > > > > This sounds like we've either mis-described the mbigen in the binding, > > or we're mis-describing the relationship with the devices. > > > > Sorry, I didn't express myself clearly. > > For example, a mbigen hardware module can support several peripheral devices > with different device ID. > > |-----------------------|- > | mbigen | > |-----------------------|- > | | | > dev1 dev2 dev3 > > To simplify the mbigen drvier, > I didn't use the whole mbigen module as a mbgien device, but use > the register collections(vector, trigger type,status etc.) corresponding > to a peripheral device as a mbigen device. > So, mbigen device is a logical conception or logical device. From the above, it sounds like the DT representation is not an accurate representation of the hardware. We should describe the _whole_ mbigen, not portions thereof. Trying to describe it piecemeal leads to problems like this one. I don't understand the rationale for describing the mbigen as separate nodes. Above you mention that we "need to define a device node for each device", but I don't see why that's necessary. Why do you believe we need an mbigen node per client device? As far as I can tell, we should be able to map an arbitrary interrupt-specifier to a particular MSI (complete with device id) within the mbigen driver. We just need to define the full set of MSIs the mbigen owns. > Now, a mbigen hardware module contains several logical mbigen device. > > ------------------------------- > |mgn_dev1 mgn_dev2 mgn_dev3 | > |-----------------------------| > | | | > dev1 dev2 dev3 > > So,mgn_dev1, mgn_dev2 and mgn_dev3 exist in same mbigen hardware module, > and,they use the same reg address region,that is adress of mbigen hardware module. > > For this case, I think the question is can we map an reg address > region more than one time? As above, I think we've mis-described the hardware. Rather than making things simpler, this has resulted in problems like this one. We should not map a reg region more than once. Even if it's technically possible to do so, I do not believe that would be the right solution here. Implicitly sharing resources (e.g. portions of the mbigen control registers) is inevitably going to lead to more problems later on. Mark.
? 2016/2/2 19:43, Mark Rutland ??: > On Tue, Feb 02, 2016 at 06:25:53PM +0800, majun (F) wrote: >> Hi Mark: >> >> ? 2016/2/1 21:25, Mark Rutland ??: >>> On Mon, Feb 01, 2016 at 09:06:38PM +0800, majun (F) wrote: >>>> >>>> >>>> ? 2016/2/1 19:57, Mark Rutland ??: >>>>> On Mon, Feb 01, 2016 at 07:44:54PM +0800, MaJun wrote: >>>>>> From: Ma Jun <majun258@huawei.com> >>>>>> >>>>>> For mbigen module, there is a special case that more than one mbigen >>>>>> device nodes use the same reg definition in DTS when these devices >>>>>> exist in the same mbigen hardware module. >>>>>> >>>>>> mbigen_dev1:intc_dev1 { >>>>>> ... >>>>>> reg = <0x0 0xc0080000 0x0 0x10000>; >>>>>> ... >>>>>> }; >>>>>> >>>>>> mbigen_dev2:intc_dev2 { >>>>>> ... >>>>>> reg = <0x0 0xc0080000 0x0 0x10000>; >>>>>> ... >>>>>> }; >>>>> >>>>> This doesn't sound right. If they exist in the same place, and have the >>>>> same reg, they _are_ the same device. >>>>> >>>>> You'll need to explain this better. >>>>> >>>> >>>> For a mbigen hardware module which connecte with several devices, >>>> because these devices has different device ID, >>>> I need to define a device node for each device in DTS file. >>>> >>>> mbigen >>>> | >>>> ------------------ >>>> | | | >>>> dev1 dev2 dev3 >>>> >>>> Because of register layout,the registers related with these devices >>>> are put together, I can't split them clearly. >>>> So, I only make these devices which connect with >>>> same mbigen hardware module usethe same address. >>> >>> This sounds like we've either mis-described the mbigen in the binding, >>> or we're mis-describing the relationship with the devices. >>> >> >> Sorry, I didn't express myself clearly. >> >> For example, a mbigen hardware module can support several peripheral devices >> with different device ID. >> >> |-----------------------|- >> | mbigen | >> |-----------------------|- >> | | | >> dev1 dev2 dev3 >> >> To simplify the mbigen drvier, >> I didn't use the whole mbigen module as a mbgien device, but use >> the register collections(vector, trigger type,status etc.) corresponding >> to a peripheral device as a mbigen device. >> So, mbigen device is a logical conception or logical device. > >>From the above, it sounds like the DT representation is not an accurate > representation of the hardware. We should describe the _whole_ mbigen, > not portions thereof. Trying to describe it piecemeal leads to problems > like this one. > > I don't understand the rationale for describing the mbigen as separate > nodes. Above you mention that we "need to define a device node for each > device", but I don't see why that's necessary. Why do you believe we > need an mbigen node per client device? > > As far as I can tell, we should be able to map an arbitrary > interrupt-specifier to a particular MSI (complete with device id) within > the mbigen driver. We just need to define the full set of MSIs the > mbigen owns. > mbigen device is a interrupt controller, it is also a ITS device for ITS module. So, we need to register the each mbigen device to ITS with a unique ID. Based on the current MSI/ITS driver, I can't register whole mbigen module which contains several mbigen devices at one time. Because they have different device ID. I don't know whether this explanation is clear or not. I think Marc can explain this well. Marc, would you please help me explain this? or, do you have any opinion about this ? >> Now, a mbigen hardware module contains several logical mbigen device. >> >> ------------------------------- >> |mgn_dev1 mgn_dev2 mgn_dev3 | >> |-----------------------------| >> | | | >> dev1 dev2 dev3 >> >> So,mgn_dev1, mgn_dev2 and mgn_dev3 exist in same mbigen hardware module, >> and,they use the same reg address region,that is adress of mbigen hardware module. >> >> For this case, I think the question is can we map an reg address >> region more than one time? > > As above, I think we've mis-described the hardware. Rather than making > things simpler, this has resulted in problems like this one. > > We should not map a reg region more than once. Even if it's technically > possible to do so, I do not believe that would be the right solution > here. Implicitly sharing resources (e.g. portions of the mbigen control > registers) is inevitably going to lead to more problems later on. Because we only need to write 1 into corresponding bit of status register to clear interrupt status during runtime,I think we don't need to worry about this. Thanks! Ma Jun > > Mark. > > . >
On Wed, Feb 03, 2016 at 10:31:52AM +0800, majun (F) wrote: > > > ? 2016/2/2 19:43, Mark Rutland ??: > > On Tue, Feb 02, 2016 at 06:25:53PM +0800, majun (F) wrote: > >> To simplify the mbigen drvier, > >> I didn't use the whole mbigen module as a mbgien device, but use > >> the register collections(vector, trigger type,status etc.) corresponding > >> to a peripheral device as a mbigen device. > >> So, mbigen device is a logical conception or logical device. > > > >>From the above, it sounds like the DT representation is not an accurate > > representation of the hardware. We should describe the _whole_ mbigen, > > not portions thereof. Trying to describe it piecemeal leads to problems > > like this one. > > > > I don't understand the rationale for describing the mbigen as separate > > nodes. Above you mention that we "need to define a device node for each > > device", but I don't see why that's necessary. Why do you believe we > > need an mbigen node per client device? > > > > As far as I can tell, we should be able to map an arbitrary > > interrupt-specifier to a particular MSI (complete with device id) within > > the mbigen driver. We just need to define the full set of MSIs the > > mbigen owns. > > > > mbigen device is a interrupt controller, it is also a ITS device for ITS module. > So, we need to register the each mbigen device to ITS with a unique ID. > Based on the current MSI/ITS driver, I can't register whole mbigen module which > contains several mbigen devices at one time. Because they have different device ID. I don't follow. You can describe this by having a top-level mbigen node featuring a reg, with a sub-node for each mbigen module with an appropriate msi-parent, e.g. mbigen { reg = < ... ... >; #interrupt-cells = <2>; #address-cells = <1>; /* module index */ module@0 { reg = <0>; msi-parent = <&its 0>; }; module@1 { reg = <1>; reg = <&its 1>; }; }; That clearly does not require the reg to be duplicated, and encodes the information you want. The infrastructure for handling that might not exist yet, but that is a Linux issue that we can fix. Marc, thoughts? I take it all interrupts within a module share the same device id? > I don't know whether this explanation is clear or not. > I think Marc can explain this well. > > Marc, would you please help me explain this? or, do you have any opinion about this ? > > >> Now, a mbigen hardware module contains several logical mbigen device. > >> > >> ------------------------------- > >> |mgn_dev1 mgn_dev2 mgn_dev3 | > >> |-----------------------------| > >> | | | > >> dev1 dev2 dev3 > >> > >> So,mgn_dev1, mgn_dev2 and mgn_dev3 exist in same mbigen hardware module, > >> and,they use the same reg address region,that is adress of mbigen hardware module. > >> > >> For this case, I think the question is can we map an reg address > >> region more than one time? > > > > As above, I think we've mis-described the hardware. Rather than making > > things simpler, this has resulted in problems like this one. > > > > We should not map a reg region more than once. Even if it's technically > > possible to do so, I do not believe that would be the right solution > > here. Implicitly sharing resources (e.g. portions of the mbigen control > > registers) is inevitably going to lead to more problems later on. > > Because we only need to write 1 into corresponding bit of status > register to clear interrupt status during runtime,I think we don't > need to worry about this. That might be currently true, but I doubt that will remain true in future. Presumably there are other control registers in the mbigen which are shared between modules. I think we do need to worry about this. Mark.
Hi Mark: sorry for late response because of chinese new year. ? 2016/2/3 19:16, Mark Rutland ??: > On Wed, Feb 03, 2016 at 10:31:52AM +0800, majun (F) wrote: >> >> >> ? 2016/2/2 19:43, Mark Rutland ??: >>> On Tue, Feb 02, 2016 at 06:25:53PM +0800, majun (F) wrote: >>>> To simplify the mbigen drvier, >>>> I didn't use the whole mbigen module as a mbgien device, but use >>>> the register collections(vector, trigger type,status etc.) corresponding >>>> to a peripheral device as a mbigen device. >>>> So, mbigen device is a logical conception or logical device. >>> >>> >From the above, it sounds like the DT representation is not an accurate >>> representation of the hardware. We should describe the _whole_ mbigen, >>> not portions thereof. Trying to describe it piecemeal leads to problems >>> like this one. >>> >>> I don't understand the rationale for describing the mbigen as separate >>> nodes. Above you mention that we "need to define a device node for each >>> device", but I don't see why that's necessary. Why do you believe we >>> need an mbigen node per client device? >>> >>> As far as I can tell, we should be able to map an arbitrary >>> interrupt-specifier to a particular MSI (complete with device id) within >>> the mbigen driver. We just need to define the full set of MSIs the >>> mbigen owns. >>> >> >> mbigen device is a interrupt controller, it is also a ITS device for ITS module. >> So, we need to register the each mbigen device to ITS with a unique ID. >> Based on the current MSI/ITS driver, I can't register whole mbigen module which >> contains several mbigen devices at one time. Because they have different device ID. > > I don't follow. > > You can describe this by having a top-level mbigen node featuring a reg, > with a sub-node for each mbigen module with an appropriate msi-parent, > e.g. > > mbigen { > reg = < ... ... >; > > #interrupt-cells = <2>; > > #address-cells = <1>; /* module index */ > > module@0 { > reg = <0>; > msi-parent = <&its 0>; > }; > > module@1 { > reg = <1>; > reg = <&its 1>; > }; > }; > > > That clearly does not require the reg to be duplicated, and encodes the > information you want. The infrastructure for handling that might not > exist yet, but that is a Linux issue that we can fix. > > Marc, thoughts? > > I take it all interrupts within a module share the same device id? > >> I don't know whether this explanation is clear or not. >> I think Marc can explain this well. >> >> Marc, would you please help me explain this? or, do you have any opinion about this ? >> >>>> Now, a mbigen hardware module contains several logical mbigen device. >>>> >>>> ------------------------------- >>>> |mgn_dev1 mgn_dev2 mgn_dev3 | >>>> |-----------------------------| >>>> | | | >>>> dev1 dev2 dev3 >>>> >>>> So,mgn_dev1, mgn_dev2 and mgn_dev3 exist in same mbigen hardware module, >>>> and,they use the same reg address region,that is adress of mbigen hardware module. >>>> >>>> For this case, I think the question is can we map an reg address >>>> region more than one time? >>> >>> As above, I think we've mis-described the hardware. Rather than making >>> things simpler, this has resulted in problems like this one. >>> >>> We should not map a reg region more than once. Even if it's technically >>> possible to do so, I do not believe that would be the right solution >>> here. Implicitly sharing resources (e.g. portions of the mbigen control >>> registers) is inevitably going to lead to more problems later on. >> >> Because we only need to write 1 into corresponding bit of status >> register to clear interrupt status during runtime,I think we don't >> need to worry about this. > > That might be currently true, but I doubt that will remain true in > future. Presumably there are other control registers in the mbigen which > are shared between modules. > I'm sure there is nothing to control except the status register even in future. Thanks MaJun > I think we do need to worry about this. > > Mark. > > . >
diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c index 4dd3eb8..b19e528 100644 --- a/drivers/irqchip/irq-mbigen.c +++ b/drivers/irqchip/irq-mbigen.c @@ -250,7 +250,7 @@ static int mbigen_device_probe(struct platform_device *pdev) mgn_chip->pdev = pdev; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - mgn_chip->base = devm_ioremap_resource(&pdev->dev, res); + mgn_chip->base = devm_ioremap(&pdev->dev, res->start, resource_size(res)); if (IS_ERR(mgn_chip->base)) return PTR_ERR(mgn_chip->base);